* [PATCH iproute2 master 2/2] geneve: add support to set flow label
From: Daniel Borkmann @ 2016-03-24 15:49 UTC (permalink / raw)
To: stephen; +Cc: netdev, Daniel Borkmann
In-Reply-To: <cover.1458833866.git.daniel@iogearbox.net>
Follow-up for kernel commit 8eb3b99554b8 ("geneve: support setting
IPv6 flow label") to allow setting the label for the device config.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
ip/iplink_geneve.c | 29 ++++++++++++++++++++++++-----
man/man8/ip-link.8.in | 6 ++++++
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/ip/iplink_geneve.c b/ip/iplink_geneve.c
index 16e70fd..685a0eb 100644
--- a/ip/iplink_geneve.c
+++ b/ip/iplink_geneve.c
@@ -18,14 +18,15 @@
static void print_explain(FILE *f)
{
fprintf(f, "Usage: ... geneve id VNI remote ADDR\n");
- fprintf(f, " [ ttl TTL ] [ tos TOS ]\n");
+ fprintf(f, " [ ttl TTL ] [ tos TOS ] [ flowlabel LABEL ]\n");
fprintf(f, " [ dstport PORT ] [ [no]external ]\n");
fprintf(f, " [ [no]udpcsum ] [ [no]udp6zerocsumtx ] [ [no]udp6zerocsumrx ]\n");
fprintf(f, "\n");
- fprintf(f, "Where: VNI := 0-16777215\n");
- fprintf(f, " ADDR := IP_ADDRESS\n");
- fprintf(f, " TOS := { NUMBER | inherit }\n");
- fprintf(f, " TTL := { 1..255 | inherit }\n");
+ fprintf(f, "Where: VNI := 0-16777215\n");
+ fprintf(f, " ADDR := IP_ADDRESS\n");
+ fprintf(f, " TOS := { NUMBER | inherit }\n");
+ fprintf(f, " TTL := { 1..255 | inherit }\n");
+ fprintf(f, " LABEL := 0-1048575\n");
}
static void explain(void)
@@ -40,6 +41,7 @@ static int geneve_parse_opt(struct link_util *lu, int argc, char **argv,
int vni_set = 0;
__u32 daddr = 0;
struct in6_addr daddr6 = IN6ADDR_ANY_INIT;
+ __u32 label = 0;
__u8 ttl = 0;
__u8 tos = 0;
__u16 dstport = 0;
@@ -90,6 +92,15 @@ static int geneve_parse_opt(struct link_util *lu, int argc, char **argv,
tos = uval;
} else
tos = 1;
+ } else if (!matches(*argv, "label") ||
+ !matches(*argv, "flowlabel")) {
+ __u32 uval;
+
+ NEXT_ARG();
+ if (get_u32(&uval, *argv, 0) ||
+ (uval & ~LABEL_MAX_MASK))
+ invarg("invalid flowlabel", *argv);
+ label = htonl(uval);
} else if (!matches(*argv, "dstport")) {
NEXT_ARG();
if (get_u16(&dstport, *argv, 0))
@@ -150,6 +161,7 @@ static int geneve_parse_opt(struct link_util *lu, int argc, char **argv,
addattr_l(n, 1024, IFLA_GENEVE_REMOTE, &daddr, 4);
if (memcmp(&daddr6, &in6addr_any, sizeof(daddr6)) != 0)
addattr_l(n, 1024, IFLA_GENEVE_REMOTE6, &daddr6, sizeof(struct in6_addr));
+ addattr32(n, 1024, IFLA_GENEVE_LABEL, label);
addattr8(n, 1024, IFLA_GENEVE_TTL, ttl);
addattr8(n, 1024, IFLA_GENEVE_TOS, tos);
if (dstport)
@@ -214,6 +226,13 @@ static void geneve_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
fprintf(f, "tos %#x ", tos);
}
+ if (tb[IFLA_GENEVE_LABEL]) {
+ __u32 label = rta_getattr_u32(tb[IFLA_GENEVE_LABEL]);
+
+ if (label)
+ fprintf(f, "flowlabel %#x ", ntohl(label));
+ }
+
if (tb[IFLA_GENEVE_PORT])
fprintf(f, "dstport %u ",
ntohs(rta_getattr_u16(tb[IFLA_GENEVE_PORT])));
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index f115c19..8055114 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -752,6 +752,8 @@ the following additional arguments are supported:
.BI ttl " TTL "
] [
.BI tos " TOS "
+] [
+.BI flowlabel " FLOWLABEL "
]
.in +8
@@ -771,6 +773,10 @@ the following additional arguments are supported:
.BI tos " TOS"
- specifies the TOS value to use in outgoing packets.
+.sp
+.BI flowlabel " FLOWLABEL"
+- specifies the flow label to use in outgoing packets.
+
.in -8
.TP
--
1.9.3
^ permalink raw reply related
* [PATCH iproute2 master 1/2] vxlan: add support to set flow label
From: Daniel Borkmann @ 2016-03-24 15:49 UTC (permalink / raw)
To: stephen; +Cc: netdev, Daniel Borkmann
In-Reply-To: <cover.1458833866.git.daniel@iogearbox.net>
Follow-up for kernel commit e7f70af111f0 ("vxlan: support setting
IPv6 flow label") to allow setting the label for the device config.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
ip/ip_common.h | 4 ++++
ip/iplink_vxlan.c | 29 ++++++++++++++++++++++++-----
man/man8/ip-link.8.in | 6 ++++++
3 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/ip/ip_common.h b/ip/ip_common.h
index 815487a..b7361a8 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -92,3 +92,7 @@ void br_dump_bridge_id(const struct ifla_bridge_id *id, char *buf, size_t len);
#ifndef INFINITY_LIFE_TIME
#define INFINITY_LIFE_TIME 0xFFFFFFFFU
#endif
+
+#ifndef LABEL_MAX_MASK
+#define LABEL_MAX_MASK 0xFFFFFU
+#endif
diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index 77aabba..bfebe09 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -24,7 +24,7 @@
static void print_explain(FILE *f)
{
fprintf(f, "Usage: ... vxlan id VNI [ { group | remote } IP_ADDRESS ] [ local ADDR ]\n");
- fprintf(f, " [ ttl TTL ] [ tos TOS ] [ dev PHYS_DEV ]\n");
+ fprintf(f, " [ ttl TTL ] [ tos TOS ] [ flowlabel LABEL ] [ dev PHYS_DEV ]\n");
fprintf(f, " [ dstport PORT ] [ srcport MIN MAX ]\n");
fprintf(f, " [ [no]learning ] [ [no]proxy ] [ [no]rsc ]\n");
fprintf(f, " [ [no]l2miss ] [ [no]l3miss ]\n");
@@ -33,10 +33,11 @@ static void print_explain(FILE *f)
fprintf(f, " [ [no]remcsumtx ] [ [no]remcsumrx ]\n");
fprintf(f, " [ [no]external ] [ gbp ]\n");
fprintf(f, "\n");
- fprintf(f, "Where: VNI := 0-16777215\n");
- fprintf(f, " ADDR := { IP_ADDRESS | any }\n");
- fprintf(f, " TOS := { NUMBER | inherit }\n");
- fprintf(f, " TTL := { 1..255 | inherit }\n");
+ fprintf(f, "Where: VNI := 0-16777215\n");
+ fprintf(f, " ADDR := { IP_ADDRESS | any }\n");
+ fprintf(f, " TOS := { NUMBER | inherit }\n");
+ fprintf(f, " TTL := { 1..255 | inherit }\n");
+ fprintf(f, " LABEL := 0-1048575\n");
}
static void explain(void)
@@ -58,6 +59,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
unsigned int link = 0;
__u8 tos = 0;
__u8 ttl = 0;
+ __u32 label = 0;
__u8 learning = 1;
__u8 proxy = 0;
__u8 rsc = 0;
@@ -146,6 +148,15 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
tos = uval;
} else
tos = 1;
+ } else if (!matches(*argv, "label") ||
+ !matches(*argv, "flowlabel")) {
+ __u32 uval;
+
+ NEXT_ARG();
+ if (get_u32(&uval, *argv, 0) ||
+ (uval & ~LABEL_MAX_MASK))
+ invarg("invalid flowlabel", *argv);
+ label = htonl(uval);
} else if (!matches(*argv, "ageing")) {
NEXT_ARG();
if (strcmp(*argv, "none") == 0)
@@ -280,6 +291,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
if (link)
addattr32(n, 1024, IFLA_VXLAN_LINK, link);
+ addattr32(n, 1024, IFLA_VXLAN_LABEL, label);
addattr8(n, 1024, IFLA_VXLAN_TTL, ttl);
addattr8(n, 1024, IFLA_VXLAN_TOS, tos);
addattr8(n, 1024, IFLA_VXLAN_LEARNING, learning);
@@ -426,6 +438,13 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
fprintf(f, "ttl %d ", ttl);
}
+ if (tb[IFLA_VXLAN_LABEL]) {
+ __u32 label = rta_getattr_u32(tb[IFLA_VXLAN_LABEL]);
+
+ if (label)
+ fprintf(f, "flowlabel %#x ", ntohl(label));
+ }
+
if (tb[IFLA_VXLAN_AGEING]) {
__u32 age = rta_getattr_u32(tb[IFLA_VXLAN_AGEING]);
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 2cd93b0..f115c19 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -396,6 +396,8 @@ the following additional arguments are supported:
] [
.BI tos " TOS "
] [
+.BI flowlabel " FLOWLABEL "
+] [
.BI dstport " PORT "
] [
.BI srcport " MIN MAX "
@@ -460,6 +462,10 @@ parameter.
- specifies the TOS value to use in outgoing packets.
.sp
+.BI flowlabel " FLOWLABEL"
+- specifies the flow label to use in outgoing packets.
+
+.sp
.BI dstport " PORT"
- specifies the UDP destination port to communicate to the remote VXLAN tunnel endpoint.
--
1.9.3
^ permalink raw reply related
* [PATCH iproute2 master 0/2] setting flow label support
From: Daniel Borkmann @ 2016-03-24 15:49 UTC (permalink / raw)
To: stephen; +Cc: netdev, Daniel Borkmann
On top of Jesse's csum patches for vxlan, geneve:
- http://patchwork.ozlabs.org/patch/599746/
- http://patchwork.ozlabs.org/patch/599747/
Thanks!
Daniel Borkmann (2):
vxlan: add support to set flow label
geneve: add support to set flow label
ip/ip_common.h | 4 ++++
ip/iplink_geneve.c | 29 ++++++++++++++++++++++++-----
ip/iplink_vxlan.c | 29 ++++++++++++++++++++++++-----
man/man8/ip-link.8.in | 12 ++++++++++++
4 files changed, 64 insertions(+), 10 deletions(-)
--
1.9.3
^ permalink raw reply
* Re: [PATCH 1/1] net: macb: remove BUG_ON() and reset the queue to handle RX errors
From: Nicolas Ferre @ 2016-03-24 15:42 UTC (permalink / raw)
To: Cyrille Pitchen, davem, linux-arm-kernel, netdev, soren.brinkmann,
narmstrong
Cc: linux-kernel
In-Reply-To: <1458830232-6159-1-git-send-email-cyrille.pitchen@atmel.com>
Le 24/03/2016 15:37, Cyrille Pitchen a écrit :
> This patch removes two BUG_ON() used to notify about RX queue corruptions
> on macb (not gem) hardware without actually handling the error.
>
> The new code skips corrupted frames but still processes faultless frames.
> Then it resets the RX queue before restarting the reception from a clean
> state.
>
> This patch is a rework of an older patch proposed by Neil Armstrong:
> http://patchwork.ozlabs.org/patch/371525/
>
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Thanks for this rework of Neil's patch that was standing for a long time
in my backlog ;-).
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Bye,
> ---
> drivers/net/ethernet/cadence/macb.c | 59 ++++++++++++++++++++++++++++++-------
> 1 file changed, 49 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 6619178ed77b..39447a337149 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -917,7 +917,10 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
> unsigned int frag_len = bp->rx_buffer_size;
>
> if (offset + frag_len > len) {
> - BUG_ON(frag != last_frag);
> + if (unlikely(frag != last_frag)) {
> + dev_kfree_skb_any(skb);
> + return -1;
> + }
> frag_len = len - offset;
> }
> skb_copy_to_linear_data_offset(skb, offset,
> @@ -945,11 +948,26 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
> return 0;
> }
>
> +static inline void macb_init_rx_ring(struct macb *bp)
> +{
> + int i;
> + dma_addr_t addr;
> +
> + addr = bp->rx_buffers_dma;
> + for (i = 0; i < RX_RING_SIZE; i++) {
> + bp->rx_ring[i].addr = addr;
> + bp->rx_ring[i].ctrl = 0;
> + addr += bp->rx_buffer_size;
> + }
> + bp->rx_ring[RX_RING_SIZE - 1].addr |= MACB_BIT(RX_WRAP);
> +}
> +
> static int macb_rx(struct macb *bp, int budget)
> {
> int received = 0;
> unsigned int tail;
> int first_frag = -1;
> + int reset_rx_queue = 0;
>
> for (tail = bp->rx_tail; budget > 0; tail++) {
> struct macb_dma_desc *desc = macb_rx_desc(bp, tail);
> @@ -972,10 +990,18 @@ static int macb_rx(struct macb *bp, int budget)
>
> if (ctrl & MACB_BIT(RX_EOF)) {
> int dropped;
> - BUG_ON(first_frag == -1);
> +
> + if (unlikely(first_frag == -1)) {
> + reset_rx_queue = 1;
> + continue;
> + }
>
> dropped = macb_rx_frame(bp, first_frag, tail);
> first_frag = -1;
> + if (unlikely(dropped < 0)) {
> + reset_rx_queue = 1;
> + continue;
> + }
> if (!dropped) {
> received++;
> budget--;
> @@ -983,6 +1009,26 @@ static int macb_rx(struct macb *bp, int budget)
> }
> }
>
> + if (unlikely(reset_rx_queue)) {
> + unsigned long flags;
> + u32 ctrl;
> +
> + netdev_err(bp->dev, "RX queue corruption: reset it\n");
> +
> + spin_lock_irqsave(&bp->lock, flags);
> +
> + ctrl = macb_readl(bp, NCR);
> + macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
> +
> + macb_init_rx_ring(bp);
> + macb_writel(bp, RBQP, bp->rx_ring_dma);
> +
> + macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
> +
> + spin_unlock_irqrestore(&bp->lock, flags);
> + return received;
> + }
> +
> if (first_frag != -1)
> bp->rx_tail = first_frag;
> else
> @@ -1523,15 +1569,8 @@ static void gem_init_rings(struct macb *bp)
> static void macb_init_rings(struct macb *bp)
> {
> int i;
> - dma_addr_t addr;
>
> - addr = bp->rx_buffers_dma;
> - for (i = 0; i < RX_RING_SIZE; i++) {
> - bp->rx_ring[i].addr = addr;
> - bp->rx_ring[i].ctrl = 0;
> - addr += bp->rx_buffer_size;
> - }
> - bp->rx_ring[RX_RING_SIZE - 1].addr |= MACB_BIT(RX_WRAP);
> + macb_init_rx_ring(bp);
>
> for (i = 0; i < TX_RING_SIZE; i++) {
> bp->queues[0].tx_ring[i].addr = 0;
>
--
Nicolas Ferre
^ permalink raw reply
* Re: [PATCH 1/1] net: macb: replace macb_writel() call by queue_writel() to update queue ISR
From: Nicolas Ferre @ 2016-03-24 15:39 UTC (permalink / raw)
To: Cyrille Pitchen, davem, linux-arm-kernel, netdev, soren.brinkmann
Cc: linux-kernel
In-Reply-To: <1458830404-6213-1-git-send-email-cyrille.pitchen@atmel.com>
Le 24/03/2016 15:40, Cyrille Pitchen a écrit :
> macb_interrupt() should not use macb_writel(bp, ISR, <value>) but only
> queue_writel(queue, ISR, <value>).
>
> There is one IRQ and one set of {ISR, IER, IDR, IMR} [1] registers per
> queue on gem hardware, though only queue0 is actually used for now to
> receive frames: other queues can already be used to transmit frames.
>
> The queue_readl() and queue_writel() helper macros are designed to access
> the relevant IRQ registers.
>
> [1]
> ISR: Interrupt Status Register
> IER: Interrupt Enable Register
> IDR: Interrupt Disable Register
> IMR: Interrupt Mask Register
>
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> Fixes: bfbb92c44670 ("net: macb: Handle the RXUBR interrupt on all devices")
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Thanks!
> ---
> drivers/net/ethernet/cadence/macb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 39447a337149..c9c6b2762a39 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -1146,7 +1146,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
> macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
>
> if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> - macb_writel(bp, ISR, MACB_BIT(RXUBR));
> + queue_writel(queue, ISR, MACB_BIT(RXUBR));
> }
>
> if (status & MACB_BIT(ISR_ROVR)) {
>
--
Nicolas Ferre
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Willy Tarreau @ 2016-03-24 15:30 UTC (permalink / raw)
To: Eric Dumazet
Cc: Tolga Ceylan, Tom Herbert, cgallek, Josh Snyder, Aaron Conole,
David S. Miller, Linux Kernel Network Developers
In-Reply-To: <1458830744.10868.72.camel@edumazet-glaptop3.roam.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 2982 bytes --]
Hi Eric,
(just lost my e-mail, trying not to forget some points)
On Thu, Mar 24, 2016 at 07:45:44AM -0700, Eric Dumazet wrote:
> On Thu, 2016-03-24 at 15:22 +0100, Willy Tarreau wrote:
> > Hi Eric,
>
> > But that means that any software making use of SO_REUSEPORT needs to
> > also implement BPF on Linux to achieve the same as what it does on
> > other OSes ? Also I found a case where a dying process would still
> > cause trouble in the accept queue, maybe it's not redistributed, I
> > don't remember, all I remember is that my traffic stopped after a
> > segfault of only one of them :-/ I'll have to dig a bit regarding
> > this.
>
> Hi Willy
>
> Problem is : If we add a SO_REUSEPORT_LISTEN_OFF, this wont work with
> BPF.
I wasn't for adding SO_REUSEPORT_LISTEN_OFF either. Instead the idea was
just to modify the score in compute_score() so that a socket which disables
SO_REUSEPORT scores less than one which still has it. The application
wishing to terminate just has to clear the SO_REUSEPORT flag and wait for
accept() reporting EAGAIN. The patch simply looked like this (copy-pasted
hence space-mangled) :
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -189,6 +189,8 @@ static inline int compute_score(struct sock *sk, struct net *net,
return -1;
score += 4;
}
+ if (sk->sk_reuseport)
+ score++;
if (sk->sk_incoming_cpu == raw_smp_processor_id())
score++;
}
> BPF makes a decision without knowing individual listeners states.
But is the decision taken without considering compute_score() ? The point
really was to be the least possibly intrusive and quite logical for the
application : "disable SO_REUSEPORT when you don't want to participate to
incoming load balancing anymore".
> Or we would need to extend BPF to access these kind of states.
> Doable certainly, but we need to be convinced it is necessary.
You know that I don't like complex designs to address simple issues if
possible :-)
> And yes, if a listener is closed while children are still in accept
> queue, we drop all the children, we do not care of redistributing them
> to another listeners. Really too complex to be worth it.
Forget this, I mixed two issues here. Yes I know that redistributing is
almost impossible, I've read that code a year ago or so and realized how
complex this would be, without providing even 100% success rate. I wasn't
speaking about redistribution of incoming queue but about an issue I've
met where when I have 4 processes bound to the same port, if one dies,
its share of incoming traffic is definitely lost. The only fix was to
restart the processes to create new listeners. But I don't remember the
conditions where I met this case, I don't even remember if it was on an
-rc kernel or a final one, so I'd prefer to discuss this only once I have
more elements.
Cheers,
Willy
[-- Attachment #2: 0001-net-make-lingering-sockets-score-less-in-compute_sco.patch --]
[-- Type: text/plain, Size: 2647 bytes --]
>From c060a5db92274402a0178d7c777a1e37c15eadb9 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
Date: Tue, 15 Dec 2015 16:40:00 +0100
Subject: net: make lingering sockets score less in compute_score()
When multiple processes use SO_REUSEPORT for a seamless restart
operation, there's a tiny window during which both the old and the new
process are bound to the same port, and there's no way for the old
process to gracefully stop receiving connections without dropping
the few that are left in the queue between the last poll() and the
shutdown() or close() operation.
Incoming connections are distributed between multiple listening sockets
in inet_lookup_listener() according to multiple criteria. The first
criterion is a score based on a number of attributes for each socket,
then a hash computation ensures that the connections are evenly
distributed between sockets of equal weight.
This patch provides a simple approach by which the old process can
simply decrease its score by disabling SO_REUSEPORT on its listening
sockets. Thus, the sockets from the new process always score higher
and are always preferred.
The old process can then safely drain incoming connections and stop
after meeting the -1 EAGAIN condition, as shown in the example below :
process A (old one) | process B (new one)
|
setsockopt(SO_REUSEPORT, 1) |
listen() >= 0 |
... |
accept() |
... | setsockopt(SO_REUSEPORT, 1)
... | listen()
From now on, both processes receive incoming connections
... | kill(process A, go_away)
setsockopt(SO_REUSEPORT, 0) | accept() >= 0
Here process A stops receiving new connections
accept() >= 0 | accept() >= 0
... |
accept() = -1 EAGAIN | accept() >= 0
close() |
exit() |
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
net/ipv4/inet_hashtables.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index ccc5980..1c950ba 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -189,6 +189,8 @@ static inline int compute_score(struct sock *sk, struct net *net,
return -1;
score += 4;
}
+ if (sk->sk_reuseport)
+ score++;
if (sk->sk_incoming_cpu == raw_smp_processor_id())
score++;
}
--
1.7.12.1
^ permalink raw reply related
* [PATCH net] net: ipv4: Multipath needs to handle unreachable nexthops
From: David Ahern @ 2016-03-24 15:25 UTC (permalink / raw)
To: netdev
Multipath route lookups should consider knowledge about next hops and not
select a hop that is known to be failed.
Example:
[h2] [h3] 15.0.0.5
| |
3| 3|
[SP1] [SP2]--+
1 2 1 2
| | /-------------+ |
| \ / |
| X |
| / \ |
| / \---------------\ |
1 2 1 2
12.0.0.2 [TOR1] 3-----------------3 [TOR2] 12.0.0.3
4 4
\ /
\ /
\ /
-------| |-----/
1 2
[TOR3]
3|
|
[h1] 12.0.0.1
host h1 with IP 12.0.0.1 has 2 paths to host h3 at 15.0.0.5:
root@h1:~# ip ro ls
...
12.0.0.0/24 dev swp1 proto kernel scope link src 12.0.0.1
15.0.0.0/16
nexthop via 12.0.0.2 dev swp1 weight 1
nexthop via 12.0.0.3 dev swp1 weight 1
...
If the link between tor3 and tor1 is down and the link between tor1
and tor2 then tor1 is effectively cut-off from h1. Yet the route lookups
in h1 are alternating between the 2 routes: ping 15.0.0.5 gets one and
ssh 15.0.0.5 gets the other. Connections that attempt to use the
12.0.0.2 nexthop fail since that neighbor is not reachable:
root@h1:~# ip neigh show
...
12.0.0.3 dev swp1 lladdr 00:02:00:00:00:1b REACHABLE
12.0.0.2 dev swp1 FAILED
...
The failed path can be avoided by considering known neighbor information
when selecting next hops. If the neighbor lookups fails we have no
knowledge about the nexthop, so give it a shot. If there is an entry
then only select the nexthop if the state is sane. This is similar to
what fib_detect_death does for some single path cases.
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
net/ipv4/fib_semantics.c | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index d97268e8ff10..28fc6700c2b1 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -1563,13 +1563,43 @@ int fib_sync_up(struct net_device *dev, unsigned int nh_flags)
void fib_select_multipath(struct fib_result *res, int hash)
{
struct fib_info *fi = res->fi;
+ struct neighbour *n;
+ int state;
for_nexthops(fi) {
if (hash > atomic_read(&nh->nh_upper_bound))
continue;
- res->nh_sel = nhsel;
- return;
+ state = NUD_NONE;
+ n = neigh_lookup(&arp_tbl, &nh->nh_gw, fi->fib_dev);
+ if (n) {
+ state = n->nud_state;
+ neigh_release(n);
+ }
+ if (!n || (state == NUD_REACHABLE) || (state & NUD_VALID)) {
+ res->nh_sel = nhsel;
+ return;
+ }
+ } endfor_nexthops(fi);
+
+ /* try the nexthops again, but covering the entries
+ * skipped by the hash
+ */
+ fi = res->fi;
+ for_nexthops(fi) {
+ if (hash <= atomic_read(&nh->nh_upper_bound))
+ continue;
+
+ state = NUD_NONE;
+ n = neigh_lookup(&arp_tbl, &nh->nh_gw, fi->fib_dev);
+ if (n) {
+ state = n->nud_state;
+ neigh_release(n);
+ }
+ if (!n || (state == NUD_REACHABLE) || (state & NUD_VALID)) {
+ res->nh_sel = nhsel;
+ return;
+ }
} endfor_nexthops(fi);
/* Race condition: route has just become dead. */
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 1/1] net: macb: remove BUG_ON() and reset the queue to handle RX errors
From: Neil Armstrong @ 2016-03-24 14:53 UTC (permalink / raw)
To: Cyrille Pitchen, nicolas.ferre, davem, linux-arm-kernel, netdev,
soren.brinkmann
Cc: linux-kernel
In-Reply-To: <1458830232-6159-1-git-send-email-cyrille.pitchen@atmel.com>
On 03/24/2016 03:37 PM, Cyrille Pitchen wrote:
> This patch removes two BUG_ON() used to notify about RX queue corruptions
> on macb (not gem) hardware without actually handling the error.
>
> The new code skips corrupted frames but still processes faultless frames.
> Then it resets the RX queue before restarting the reception from a clean
> state.
>
> This patch is a rework of an older patch proposed by Neil Armstrong:
> http://patchwork.ozlabs.org/patch/371525/
>
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> ---
> drivers/net/ethernet/cadence/macb.c | 59 ++++++++++++++++++++++++++++++-------
> 1 file changed, 49 insertions(+), 10 deletions(-)
Hi Cyrille,
Thanks for the rework, we solved this situation by moving the descriptors to
an internal RAM with lower latencies, we suspected our AHB-AXI bridge + DDR Controller
to delay the descriptor writes while the interrupt was handled in the meantime.
But the error case was still not handled.
Acked-by: Neil Armstrong <narmstrong@baylibre.com>
Thanks,
Neil
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Eric Dumazet @ 2016-03-24 14:45 UTC (permalink / raw)
To: Willy Tarreau
Cc: Tolga Ceylan, Tom Herbert, cgallek, Josh Snyder, Aaron Conole,
David S. Miller, Linux Kernel Network Developers
In-Reply-To: <20160324142222.GB7237@1wt.eu>
On Thu, 2016-03-24 at 15:22 +0100, Willy Tarreau wrote:
> Hi Eric,
> But that means that any software making use of SO_REUSEPORT needs to
> also implement BPF on Linux to achieve the same as what it does on
> other OSes ? Also I found a case where a dying process would still
> cause trouble in the accept queue, maybe it's not redistributed, I
> don't remember, all I remember is that my traffic stopped after a
> segfault of only one of them :-/ I'll have to dig a bit regarding
> this.
Hi Willy
Problem is : If we add a SO_REUSEPORT_LISTEN_OFF, this wont work with
BPF.
BPF makes a decision without knowing individual listeners states.
Or we would need to extend BPF to access these kind of states.
Doable certainly, but we need to be convinced it is necessary.
And yes, if a listener is closed while children are still in accept
queue, we drop all the children, we do not care of redistributing them
to another listeners. Really too complex to be worth it.
For example, we could probably revert
70da268b569d32a9fddeea85dc18043de9d89f89
("net: SO_INCOMING_CPU setsockopt() support") as this can be handled by
BPF as well, and would remove extra tests in fast path (when
SO_REUSEPORT is not used at all)
^ permalink raw reply
* [PATCH 1/1] net: macb: replace macb_writel() call by queue_writel() to update queue ISR
From: Cyrille Pitchen @ 2016-03-24 14:40 UTC (permalink / raw)
To: nicolas.ferre, davem, linux-arm-kernel, netdev, soren.brinkmann
Cc: linux-kernel, Cyrille Pitchen
macb_interrupt() should not use macb_writel(bp, ISR, <value>) but only
queue_writel(queue, ISR, <value>).
There is one IRQ and one set of {ISR, IER, IDR, IMR} [1] registers per
queue on gem hardware, though only queue0 is actually used for now to
receive frames: other queues can already be used to transmit frames.
The queue_readl() and queue_writel() helper macros are designed to access
the relevant IRQ registers.
[1]
ISR: Interrupt Status Register
IER: Interrupt Enable Register
IDR: Interrupt Disable Register
IMR: Interrupt Mask Register
Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Fixes: bfbb92c44670 ("net: macb: Handle the RXUBR interrupt on all devices")
---
drivers/net/ethernet/cadence/macb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 39447a337149..c9c6b2762a39 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -1146,7 +1146,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- macb_writel(bp, ISR, MACB_BIT(RXUBR));
+ queue_writel(queue, ISR, MACB_BIT(RXUBR));
}
if (status & MACB_BIT(ISR_ROVR)) {
--
1.8.2.2
^ permalink raw reply related
* [PATCH 1/1] net: macb: remove BUG_ON() and reset the queue to handle RX errors
From: Cyrille Pitchen @ 2016-03-24 14:37 UTC (permalink / raw)
To: nicolas.ferre, davem, linux-arm-kernel, netdev, soren.brinkmann,
narmstrong
Cc: linux-kernel, Cyrille Pitchen
This patch removes two BUG_ON() used to notify about RX queue corruptions
on macb (not gem) hardware without actually handling the error.
The new code skips corrupted frames but still processes faultless frames.
Then it resets the RX queue before restarting the reception from a clean
state.
This patch is a rework of an older patch proposed by Neil Armstrong:
http://patchwork.ozlabs.org/patch/371525/
Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
drivers/net/ethernet/cadence/macb.c | 59 ++++++++++++++++++++++++++++++-------
1 file changed, 49 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 6619178ed77b..39447a337149 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -917,7 +917,10 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
unsigned int frag_len = bp->rx_buffer_size;
if (offset + frag_len > len) {
- BUG_ON(frag != last_frag);
+ if (unlikely(frag != last_frag)) {
+ dev_kfree_skb_any(skb);
+ return -1;
+ }
frag_len = len - offset;
}
skb_copy_to_linear_data_offset(skb, offset,
@@ -945,11 +948,26 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
return 0;
}
+static inline void macb_init_rx_ring(struct macb *bp)
+{
+ int i;
+ dma_addr_t addr;
+
+ addr = bp->rx_buffers_dma;
+ for (i = 0; i < RX_RING_SIZE; i++) {
+ bp->rx_ring[i].addr = addr;
+ bp->rx_ring[i].ctrl = 0;
+ addr += bp->rx_buffer_size;
+ }
+ bp->rx_ring[RX_RING_SIZE - 1].addr |= MACB_BIT(RX_WRAP);
+}
+
static int macb_rx(struct macb *bp, int budget)
{
int received = 0;
unsigned int tail;
int first_frag = -1;
+ int reset_rx_queue = 0;
for (tail = bp->rx_tail; budget > 0; tail++) {
struct macb_dma_desc *desc = macb_rx_desc(bp, tail);
@@ -972,10 +990,18 @@ static int macb_rx(struct macb *bp, int budget)
if (ctrl & MACB_BIT(RX_EOF)) {
int dropped;
- BUG_ON(first_frag == -1);
+
+ if (unlikely(first_frag == -1)) {
+ reset_rx_queue = 1;
+ continue;
+ }
dropped = macb_rx_frame(bp, first_frag, tail);
first_frag = -1;
+ if (unlikely(dropped < 0)) {
+ reset_rx_queue = 1;
+ continue;
+ }
if (!dropped) {
received++;
budget--;
@@ -983,6 +1009,26 @@ static int macb_rx(struct macb *bp, int budget)
}
}
+ if (unlikely(reset_rx_queue)) {
+ unsigned long flags;
+ u32 ctrl;
+
+ netdev_err(bp->dev, "RX queue corruption: reset it\n");
+
+ spin_lock_irqsave(&bp->lock, flags);
+
+ ctrl = macb_readl(bp, NCR);
+ macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
+
+ macb_init_rx_ring(bp);
+ macb_writel(bp, RBQP, bp->rx_ring_dma);
+
+ macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
+
+ spin_unlock_irqrestore(&bp->lock, flags);
+ return received;
+ }
+
if (first_frag != -1)
bp->rx_tail = first_frag;
else
@@ -1523,15 +1569,8 @@ static void gem_init_rings(struct macb *bp)
static void macb_init_rings(struct macb *bp)
{
int i;
- dma_addr_t addr;
- addr = bp->rx_buffers_dma;
- for (i = 0; i < RX_RING_SIZE; i++) {
- bp->rx_ring[i].addr = addr;
- bp->rx_ring[i].ctrl = 0;
- addr += bp->rx_buffer_size;
- }
- bp->rx_ring[RX_RING_SIZE - 1].addr |= MACB_BIT(RX_WRAP);
+ macb_init_rx_ring(bp);
for (i = 0; i < TX_RING_SIZE; i++) {
bp->queues[0].tx_ring[i].addr = 0;
--
1.8.2.2
^ permalink raw reply related
* Re: [RFD] workqueue: WQ_MEM_RECLAIM usage in network drivers
From: Johannes Berg @ 2016-03-24 14:22 UTC (permalink / raw)
To: Tejun Heo, J. Bruce Fields
Cc: Jeff Layton, David S. Miller, Trond Myklebust, Anna Schumaker,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, Amitoj Kaur Chawla,
kernel-team-b10kYP2dOMg, Johannes Weiner, Eva Rachel Retuya,
Bhaktipriya Shridhar, linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160320185507.GT20028-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
On Sun, 2016-03-20 at 14:55 -0400, Tejun Heo wrote:
> If everything else is working, I'd be happy to throw in
> WQ_MEM_RECLAIM but I really don't want to add it if it doesn't
> actually achieve the goal. Can a wireless person chime in here?
>
I think for many wireless devices the workqueue, like for iwldvm that
was just changed, isn't in the packet path and thus less relevant.
For some, like SDIO based chips, it's more likely to be in the packet
path, so it would make sense to keep it.
Of course there's always a possibility of getting disconnected and then
not being able to re-establish the connection in memory pressure, but
that's not something we can control/predict (the disconnection). Can of
course also happen with wired if somebody pulls out the cable, but it's
less reliant on memory allocations to get back to working there, I
guess :)
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Willy Tarreau @ 2016-03-24 14:22 UTC (permalink / raw)
To: Eric Dumazet
Cc: Tolga Ceylan, Tom Herbert, cgallek, Josh Snyder, Aaron Conole,
David S. Miller, Linux Kernel Network Developers
In-Reply-To: <1458828813.10868.65.camel@edumazet-glaptop3.roam.corp.google.com>
Hi Eric,
On Thu, Mar 24, 2016 at 07:13:33AM -0700, Eric Dumazet wrote:
> On Thu, 2016-03-24 at 07:12 +0100, Willy Tarreau wrote:
> > Hi,
> >
> > On Wed, Mar 23, 2016 at 10:10:06PM -0700, Tolga Ceylan wrote:
> > > I apologize for not properly following up on this. I had the
> > > impression that we did not want to merge my original patch and then I
> > > also noticed that it fails to keep the hash consistent. Recently, I
> > > read the follow ups on it as well as Willy's patch/proposals.
> > >
> > > Is there any update on Willy's SO_REUSEPORT patch? IMHO, it solves the
> > > problem and it is simpler than adding new sock option.
> >
> > no, Craig's changes were merged, and I haven't checked yet if my patch
> > needs to be rebased or still applies. Feel free to check it and resubmit
> > if you have time.
>
> No need for a patch AFAIK.
>
> BPF solution is generic enough.
>
> All user space needs to do is to update the BPF filter so that the
> listener needing to be dismantled does not receive any new packet.
But that means that any software making use of SO_REUSEPORT needs to
also implement BPF on Linux to achieve the same as what it does on
other OSes ? Also I found a case where a dying process would still
cause trouble in the accept queue, maybe it's not redistributed, I
don't remember, all I remember is that my traffic stopped after a
segfault of only one of them :-/ I'll have to dig a bit regarding
this.
Thanks,
Willy
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Eric Dumazet @ 2016-03-24 14:13 UTC (permalink / raw)
To: Willy Tarreau
Cc: Tolga Ceylan, Tom Herbert, cgallek, Josh Snyder, Aaron Conole,
David S. Miller, Linux Kernel Network Developers
In-Reply-To: <20160324061222.GA6807@1wt.eu>
On Thu, 2016-03-24 at 07:12 +0100, Willy Tarreau wrote:
> Hi,
>
> On Wed, Mar 23, 2016 at 10:10:06PM -0700, Tolga Ceylan wrote:
> > I apologize for not properly following up on this. I had the
> > impression that we did not want to merge my original patch and then I
> > also noticed that it fails to keep the hash consistent. Recently, I
> > read the follow ups on it as well as Willy's patch/proposals.
> >
> > Is there any update on Willy's SO_REUSEPORT patch? IMHO, it solves the
> > problem and it is simpler than adding new sock option.
>
> no, Craig's changes were merged, and I haven't checked yet if my patch
> needs to be rebased or still applies. Feel free to check it and resubmit
> if you have time.
No need for a patch AFAIK.
BPF solution is generic enough.
All user space needs to do is to update the BPF filter so that the
listener needing to be dismantled does not receive any new packet.
^ permalink raw reply
* Re: [PATCH] net: phy: at803x: Request 'reset' GPIO only for AT8030 PHY
From: Sergei Shtylyov @ 2016-03-24 13:40 UTC (permalink / raw)
To: Sebastian Frias, Uwe Kleine-König
Cc: Daniel Mack, David S. Miller, netdev, lkml, mason,
Florian Fainelli, Mans Rullgard, Fabio Estevam,
Martin Blumenstingl, Linus Walleij
In-Reply-To: <56F3BD28.4080908@laposte.net>
On 03/24/2016 01:10 PM, Sebastian Frias wrote:
>>> What I don't understand is why the link_change_notify() method ptr is
>>> populated for all 3 supported chips while only being needed on 8030...
>>
>> You are right.
>
> I made the patch but I'm unsure about it because it could conflict with
> yours.
> I mean, I think you submitted a patch to change the GPIO handling on the
> link_change_notify() function, right?
> Well, if we only register the callback for the AT8030, then there is no
> more need for the callback to check the PHY ID.
> However, if I change that, the whole block moves as I remove one
> indentation level (the one required by the PHY ID check).
>
> Any suggestions on how to create a patch that won't conflict? I probably
> need to use a tree that already has your patch applied.
My patch is already in Linus' tree, so should be merged back from net.git
into net-next.git
soon. I suggest that you wait until net-next is open again.
> Best regards,
>
> Sebastian
MBR, Sergei
^ permalink raw reply
* Re: [PATCH 0/3] Control ethernet PHY LEDs via LED subsystem
From: Andrew Lunn @ 2016-03-24 13:29 UTC (permalink / raw)
To: Vishal Thanki; +Cc: Florian Fainelli, Matus Ujhelyi, netdev
In-Reply-To: <CAC3a_SBjvQFT5XP2bH3TO_4wHnjZ3a3+XvfBy8StxQieEsavQQ@mail.gmail.com>
On Wed, Mar 23, 2016 at 10:24:45PM +0100, Vishal Thanki wrote:
> On Wed, Mar 23, 2016 at 10:10 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > On Wed, Mar 23, 2016 at 09:24:00PM +0100, Vishal Thanki wrote:
> >> Hi,
> >>
> >> > My suggestion was that the hardware needs to control the LEDs. You
> >> > have software doing it. You might be able to do this with the PHY
> >> > state machine for link. But activity is never going to be accepted if
> >> > software control.
> >> >
> >> > The LED trigger attached to an LED should be used to configure the
> >> > hardware to drive the LED as wanted.
> >> >
> >>
> >> The eth-phy-activity trigger uses the blink_set which I think uses the
> >> hardware acceleration if available. I am not sure how to handles LEDs
> >> which does not have hardware acceleration for this (eth-phy-activity)
> >> trigger.
> >
> > We want the LED to blink on activity, real packets coming in and
> > out. The PHY can do this, so let the PHY control the LED. In this
> > case, the trigger is just mechanism for the user to say what the LED
> > should be used for. The trigger is not itself controlling the LED, it
> > has no idea about packets coming and going.
> >
>
> Yes, I understand that. But PHY can only control the LEDs attached to
> it directly. The at803x led driver configures the PHY to blink the
> activity LED based on traffic but I think it is not possible for PHY
> to control other LEDs in system, for example some other LEDs in system
> controlled only via GPIO. In such cases, putting PHY activity trigger
> on the GPIO LEDs would not make sense. Correct me if I am wrong.
Hi Vishal
All correct. Which is why i said in my original email, you need to
extend the LED core to associate triggers to LEDs. You can then
associate the eth-phy-activity trigger to only PHY leds which can
implement that functionality.
drivers/leds/led-triggers.c contains a global list
LIST_HEAD(trigger_list) which triggers get added to using
led_trigger_register(). You could add a second list to the
led_classdev structure, and add an led_trigger_register_to_led()
function which registers a trigger to a specific LED, on its own
trigger list. led_trigger_store() and led_trigger_show() would use
both lists.
Andrew
^ permalink raw reply
* Re: [PATCH] net: mlxsw: avoid unused variable warnings
From: Arnd Bergmann @ 2016-03-24 13:14 UTC (permalink / raw)
To: David Miller; +Cc: andrew, jiri, idosch, eladr, netdev, linux-kernel
In-Reply-To: <20160323.143805.310789862506977932.davem@davemloft.net>
On Wednesday 23 March 2016 14:38:05 David Miller wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> Date: Wed, 23 Mar 2016 17:51:11 +0100
>
> > On Wed, Mar 23, 2016 at 05:37:38PM +0100, Arnd Bergmann wrote:
> >> dev_dbg_ratelimited() is a macro that ignores its arguments when DEBUG is
> >> not set, which can lead to unused variable warnings:
> >>
> >> ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_sdq_handle':
> >> ethernet/mellanox/mlxsw/pci.c:646:18: warning: unused variable 'pdev' [-Wunused-variable]
> >> ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_rdq_handle':
> >> ethernet/mellanox/mlxsw/pci.c:671:18: warning: unused variable 'pdev' [-Wunused-variable]
> >>
> >> This changes the mlxsw driver to remove the local variables we get
> >> warnings for and instead pass the device directly into the API.
> >
> > Hi Arnd
> >
> > Would it not be better to fix the macro?
> >
> > I think the issue is that dev_dbg_ratelimited calls no_printk(),
> > without making use of dev. So how about:
> >
> > #define dev_dbg_ratelimited(dev, fmt, ...) \
> > ({ \
> > if (0) \
> > dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
> > })
> >
> > This follows the pattern for other macros for when DEBUG is not defined.
>
> Yeah, this is probably a better way to fix this problem.
Makes sense. I was thrown off by how a related patch recently
modified the no_printk() definition in Fixes: fe22cd9b7c98 ("printk:
help pr_debug and pr_devel to optimize out arguments") around the
same time I ran into the problem in the mlxsw driver.
I'll test build the patch below for a while and submit that if it doesn't
cause any other problems.
diff --git a/include/linux/device.h b/include/linux/device.h
index 002c59728dbe..07f74c246cac 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1293,8 +1293,11 @@ do { \
dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
} while (0)
#else
-#define dev_dbg_ratelimited(dev, fmt, ...) \
- no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#define dev_dbg_ratelimited(dev, fmt, ...) \
+do { \
+ if (0) \
+ dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
+} while (0)
#endif
#ifdef VERBOSE_DEBUG
Thanks,
Arnd
^ permalink raw reply related
* Re: [PATCH] stmmac: Fix phy without MDIO subnode
From: Sergei Shtylyov @ 2016-03-24 13:00 UTC (permalink / raw)
To: John Keeping, Giuseppe Cavallaro; +Cc: Gabriel Fernandez, netdev, linux-kernel
In-Reply-To: <1458817012-14293-1-git-send-email-john@metanate.com>
Hello.
On 3/24/2016 1:56 PM, John Keeping wrote:
> Since commit 88f8b1bb41c6 ("stmmac: Fix 'eth0: No PHY found'
> regression") we no longer allocate mdio_bus_data unless there is a MDIO
> subnode. This breaks the ethernet on the Radxa Rock2 (using
> rk3288-rock2-square.dts) which does not have an MDIO subnode.
>
> That commit was correct that the phy_bus_name test is unhelpful since we
> allocate "plat" in the same function and never set phy_bus_name so let's
> just drop the test which restores the previous behaviour.
>
> Fixes: 88f8b1bb41c6 ("stmmac: Fix 'eth0: No PHY found' regression")
> Signed-off-by: John Keeping <john@metanate.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index dcbd2a1..e0fa060 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -189,7 +189,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
> if (of_property_read_u32(np, "snps,phy-addr", &plat->phy_addr) == 0)
> dev_warn(&pdev->dev, "snps,phy-addr property is deprecated\n");
>
> - if ((plat->phy_node && !of_phy_is_fixed_link(np)) || !plat->mdio_node)
> + if ((plat->phy_node && !of_phy_is_fixed_link(np)))
Too many parens...
MBR, Sergei
^ permalink raw reply
* Re: [PATCH] stmmac: Fix phy without MDIO subnode
From: Giuseppe CAVALLARO @ 2016-03-24 12:56 UTC (permalink / raw)
To: John Keeping; +Cc: Gabriel Fernandez, netdev, linux-kernel
In-Reply-To: <1458817012-14293-1-git-send-email-john@metanate.com>
Hi John
This should be fixed by some work done some days
ago and not yet committed.
Pls see "stmmac: MDIO fixes" patch-set and let me know
if ok on your side.
Regards
Peppe
On 3/24/2016 11:56 AM, John Keeping wrote:
> Since commit 88f8b1bb41c6 ("stmmac: Fix 'eth0: No PHY found'
> regression") we no longer allocate mdio_bus_data unless there is a MDIO
> subnode. This breaks the ethernet on the Radxa Rock2 (using
> rk3288-rock2-square.dts) which does not have an MDIO subnode.
>
> That commit was correct that the phy_bus_name test is unhelpful since we
> allocate "plat" in the same function and never set phy_bus_name so let's
> just drop the test which restores the previous behaviour.
>
> Fixes: 88f8b1bb41c6 ("stmmac: Fix 'eth0: No PHY found' regression")
> Signed-off-by: John Keeping <john@metanate.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index dcbd2a1..e0fa060 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -189,7 +189,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
> if (of_property_read_u32(np, "snps,phy-addr", &plat->phy_addr) == 0)
> dev_warn(&pdev->dev, "snps,phy-addr property is deprecated\n");
>
> - if ((plat->phy_node && !of_phy_is_fixed_link(np)) || !plat->mdio_node)
> + if ((plat->phy_node && !of_phy_is_fixed_link(np)))
> plat->mdio_bus_data = NULL;
> else
> plat->mdio_bus_data =
>
^ permalink raw reply
* Re: [RFCv2 2/3] ath10k: report per-station tx/rate rates to mac80211
From: Michal Kazior @ 2016-03-24 12:31 UTC (permalink / raw)
To: Mohammed Shafi Shajakhan
Cc: linux-wireless, Felix Fietkau, Emmanuel Grumbach,
Network Development, Dave Taht,
ath10k-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
codel-JXvr2/1DY2fm6VMwtOF2vx4hnT+Y9+D1,
make-wifi-fast-JXvr2/1DY2fm6VMwtOF2vx4hnT+Y9+D1, Johannes Berg,
Tim Shepard
In-Reply-To: <20160324122317.GB30872@atheros-ThinkPad-T61>
On 24 March 2016 at 13:23, Mohammed Shafi Shajakhan
<mohammed-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> On Thu, Mar 24, 2016 at 08:49:12AM +0100, Michal Kazior wrote:
>> On 24 March 2016 at 08:19, Mohammed Shafi Shajakhan
>> <mohammed-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>> > Hi Michal,
>> >
>> > On Wed, Mar 16, 2016 at 11:17:57AM +0100, Michal Kazior wrote:
>> >> The rate control is offloaded by firmware so it's
>> >> challanging to provide expected throughput value
>> >> for given station.
>> >>
>> >> This approach is naive as it reports last tx rate
>> >> used for given station as provided by firmware
>> >> stat event.
>> >>
>> >> This should be sufficient for airtime estimation
>> >> used for fq-codel-in-mac80211 tx scheduling
>> >> purposes now.
>> >>
>> >> This patch uses a very hacky way to get the stats.
>> >> This is sufficient for proof-of-concept but must
>> >> be cleaned up properly eventually.
>> >>
>> >> Signed-off-by: Michal Kazior <michal.kazior-++hxYGjEMp0AvxtiuMwx3w@public.gmane.org>
>> >> ---
>> >> drivers/net/wireless/ath/ath10k/core.h | 5 +++
>> >> drivers/net/wireless/ath/ath10k/debug.c | 61 +++++++++++++++++++++++++++++----
>> >> drivers/net/wireless/ath/ath10k/mac.c | 26 ++++++++------
>> >> drivers/net/wireless/ath/ath10k/wmi.h | 2 +-
>> >> 4 files changed, 76 insertions(+), 18 deletions(-)
>> >>
>> >> diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
>> >> index 23ba03fb7a5f..3f76669d44cf 100644
>> >> --- a/drivers/net/wireless/ath/ath10k/core.h
>> >> +++ b/drivers/net/wireless/ath/ath10k/core.h
>> >> @@ -331,6 +331,9 @@ struct ath10k_sta {
>> >> /* protected by conf_mutex */
>> >> bool aggr_mode;
>> >> u64 rx_duration;
>> >> +
>> >> + u32 tx_rate_kbps;
>> >> + u32 rx_rate_kbps;
>> >> #endif
>> >> };
>> >>
>> >> @@ -372,6 +375,8 @@ struct ath10k_vif {
>> >> s8 def_wep_key_idx;
>> >>
>> >> u16 tx_seq_no;
>> >> + u32 tx_rate_kbps;
>> >> + u32 rx_rate_kbps;
>> >>
>> >> union {
>> >> struct {
>> >> diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
>> >> index 076d29b53ddf..cc7ebf04ae00 100644
>> >> --- a/drivers/net/wireless/ath/ath10k/debug.c
>> >> +++ b/drivers/net/wireless/ath/ath10k/debug.c
>> >> @@ -316,6 +316,58 @@ static void ath10k_debug_fw_stats_reset(struct ath10k *ar)
>> >> spin_unlock_bh(&ar->data_lock);
>> >> }
>> >>
>> >> +static void ath10k_mac_update_txrx_rate_iter(void *data,
>> >> + u8 *mac,
>> >> + struct ieee80211_vif *vif)
>> >> +{
>> >> + struct ath10k_fw_stats_peer *peer = data;
>> >> + struct ath10k_vif *arvif;
>> >> +
>> >> + if (memcmp(vif->addr, peer->peer_macaddr, ETH_ALEN))
>> >> + return;
>> >> +
>> >> + arvif = (void *)vif->drv_priv;
>> >> + arvif->tx_rate_kbps = peer->peer_tx_rate;
>> >> + arvif->rx_rate_kbps = peer->peer_rx_rate;
>> >> +}
>> >> +
>> >> +static void ath10k_mac_update_txrx_rate(struct ath10k *ar,
>> >> + struct ath10k_fw_stats *stats)
>> >> +{
>> >> + struct ieee80211_hw *hw = ar->hw;
>> >> + struct ath10k_fw_stats_peer *peer;
>> >> + struct ath10k_sta *arsta;
>> >> + struct ieee80211_sta *sta;
>> >> + const u8 *localaddr = NULL;
>> >> +
>> >> + rcu_read_lock();
>> >> +
>> >> + list_for_each_entry(peer, &stats->peers, list) {
>> >> + /* This doesn't account for multiple STA connected on different
>> >> + * vifs. Unfortunately there's no way to derive that from the available
>> >> + * information.
>> >> + */
>> >> + sta = ieee80211_find_sta_by_ifaddr(hw,
>> >> + peer->peer_macaddr,
>> >> + localaddr);
>> >> + if (!sta) {
>> >> + /* This tries to update multicast rates */
>> >> + ieee80211_iterate_active_interfaces_atomic(
>> >> + hw,
>> >> + IEEE80211_IFACE_ITER_NORMAL,
>> >> + ath10k_mac_update_txrx_rate_iter,
>> >> + peer);
>> >> + continue;
>> >> + }
>> >> +
>> >> + arsta = (void *)sta->drv_priv;
>> >> + arsta->tx_rate_kbps = peer->peer_tx_rate;
>> >> + arsta->rx_rate_kbps = peer->peer_rx_rate;
>> >> + }
>> >> +
>> >> + rcu_read_unlock();
>> >> +}
>> >> +
>> >> void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
>> >> {
>> >> struct ath10k_fw_stats stats = {};
>> >> @@ -335,6 +387,8 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
>> >> goto free;
>> >> }
>> >>
>> >> + ath10k_mac_update_txrx_rate(ar, &stats);
>> >> +
>> >> /* Stat data may exceed htc-wmi buffer limit. In such case firmware
>> >> * splits the stats data and delivers it in a ping-pong fashion of
>> >> * request cmd-update event.
>> >> @@ -351,13 +405,6 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
>> >> if (peer_stats_svc)
>> >> ath10k_sta_update_rx_duration(ar, &stats.peers);
>> >>
>> >> - if (ar->debug.fw_stats_done) {
>> >> - if (!peer_stats_svc)
>> >> - ath10k_warn(ar, "received unsolicited stats update event\n");
>> >> -
>> >> - goto free;
>> >> - }
>> >> -
>> >
>> > [shafi] As you had suggested previously, should we completely clean up this ping
>> > - pong response approach for f/w stats, (or) this should be retained to support
>> > backward compatibility and also for supporting ping - pong response when user
>> > cats for fw-stats (via debugfs) (i did see in the commit message this needs to
>> > be cleaned up)
>>
>> I think it makes sense to remove the ping-pong logic and rely on
>> periodic updates alone, including fw_stats and ethstats handling.
>>
>>
>> >> - if (test_bit(WMI_SERVICE_PEER_STATS, ar->wmi.svc_map)) {
>> >> - param = ar->wmi.pdev_param->peer_stats_update_period;
>> >> - ret = ath10k_wmi_pdev_set_param(ar, param,
>> >> - PEER_DEFAULT_STATS_UPDATE_PERIOD);
>> >> - if (ret) {
>> >> - ath10k_warn(ar,
>> >> - "failed to set peer stats period : %d\n",
>> >> - ret);
>> >> - goto err_core_stop;
>> >> - }
>> >> + param = ar->wmi.pdev_param->peer_stats_update_period;
>> >> + ret = ath10k_wmi_pdev_set_param(ar, param,
>> >> + PEER_DEFAULT_STATS_UPDATE_PERIOD);
>> >> + if (ret) {
>> >> + ath10k_warn(ar,
>> >> + "failed to set peer stats period : %d\n",
>> >> + ret);
>> >> + goto err_core_stop;
>> >> }
>> >
>> > [shafi] If i am correct this change requires 'PEER_STATS' to be enabled by
>> > default.
>>
>> No, it does not. Periodic stats have been available since forever.
>
> [shafi] Michal, sorry i was talking about enabling WMI_PEER_STATS feature for
> 10.2, and we have a patch pushed recently to reduce the number of peers if
> 'WMI_PEER_STATS' feature is enabled(avoiding f/w crash due to memory
> constraints) . But this patch requires the feature to be
> turned ON always (with periodic stats update as well for evey 100ms). Please
> correct me if my understanding is wrong.
Periodic stats and extended stats are two separate things.
WMI_PEER_STATS is a feature which prompts firmware to gather more
statistics and then report them to host via stat update event and
includes, e.g. rx_duration. Due to how rx_duration was designed in
firmware it needs to be combined with reading out the stats often to
make it usable. Periodic stats were used instead of explicit
ping-pong.
Periodic stats is just one of two ways (the other being ping-pong)
asking firmware for stat update event. It has been in firmware since
forever.
Michał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFCv2 2/3] ath10k: report per-station tx/rate rates to mac80211
From: Mohammed Shafi Shajakhan @ 2016-03-24 12:23 UTC (permalink / raw)
To: Michal Kazior
Cc: linux-wireless, Felix Fietkau, Emmanuel Grumbach,
Network Development, Dave Taht, ath10k@lists.infradead.org, codel,
make-wifi-fast, Johannes Berg, Tim Shepard
In-Reply-To: <CA+BoTQmRBog+oSMCKUUzTV1Jm_Mip0ZA9mnpkZ14hDUq32Dszg@mail.gmail.com>
On Thu, Mar 24, 2016 at 08:49:12AM +0100, Michal Kazior wrote:
> On 24 March 2016 at 08:19, Mohammed Shafi Shajakhan
> <mohammed@codeaurora.org> wrote:
> > Hi Michal,
> >
> > On Wed, Mar 16, 2016 at 11:17:57AM +0100, Michal Kazior wrote:
> >> The rate control is offloaded by firmware so it's
> >> challanging to provide expected throughput value
> >> for given station.
> >>
> >> This approach is naive as it reports last tx rate
> >> used for given station as provided by firmware
> >> stat event.
> >>
> >> This should be sufficient for airtime estimation
> >> used for fq-codel-in-mac80211 tx scheduling
> >> purposes now.
> >>
> >> This patch uses a very hacky way to get the stats.
> >> This is sufficient for proof-of-concept but must
> >> be cleaned up properly eventually.
> >>
> >> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> >> ---
> >> drivers/net/wireless/ath/ath10k/core.h | 5 +++
> >> drivers/net/wireless/ath/ath10k/debug.c | 61 +++++++++++++++++++++++++++++----
> >> drivers/net/wireless/ath/ath10k/mac.c | 26 ++++++++------
> >> drivers/net/wireless/ath/ath10k/wmi.h | 2 +-
> >> 4 files changed, 76 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
> >> index 23ba03fb7a5f..3f76669d44cf 100644
> >> --- a/drivers/net/wireless/ath/ath10k/core.h
> >> +++ b/drivers/net/wireless/ath/ath10k/core.h
> >> @@ -331,6 +331,9 @@ struct ath10k_sta {
> >> /* protected by conf_mutex */
> >> bool aggr_mode;
> >> u64 rx_duration;
> >> +
> >> + u32 tx_rate_kbps;
> >> + u32 rx_rate_kbps;
> >> #endif
> >> };
> >>
> >> @@ -372,6 +375,8 @@ struct ath10k_vif {
> >> s8 def_wep_key_idx;
> >>
> >> u16 tx_seq_no;
> >> + u32 tx_rate_kbps;
> >> + u32 rx_rate_kbps;
> >>
> >> union {
> >> struct {
> >> diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
> >> index 076d29b53ddf..cc7ebf04ae00 100644
> >> --- a/drivers/net/wireless/ath/ath10k/debug.c
> >> +++ b/drivers/net/wireless/ath/ath10k/debug.c
> >> @@ -316,6 +316,58 @@ static void ath10k_debug_fw_stats_reset(struct ath10k *ar)
> >> spin_unlock_bh(&ar->data_lock);
> >> }
> >>
> >> +static void ath10k_mac_update_txrx_rate_iter(void *data,
> >> + u8 *mac,
> >> + struct ieee80211_vif *vif)
> >> +{
> >> + struct ath10k_fw_stats_peer *peer = data;
> >> + struct ath10k_vif *arvif;
> >> +
> >> + if (memcmp(vif->addr, peer->peer_macaddr, ETH_ALEN))
> >> + return;
> >> +
> >> + arvif = (void *)vif->drv_priv;
> >> + arvif->tx_rate_kbps = peer->peer_tx_rate;
> >> + arvif->rx_rate_kbps = peer->peer_rx_rate;
> >> +}
> >> +
> >> +static void ath10k_mac_update_txrx_rate(struct ath10k *ar,
> >> + struct ath10k_fw_stats *stats)
> >> +{
> >> + struct ieee80211_hw *hw = ar->hw;
> >> + struct ath10k_fw_stats_peer *peer;
> >> + struct ath10k_sta *arsta;
> >> + struct ieee80211_sta *sta;
> >> + const u8 *localaddr = NULL;
> >> +
> >> + rcu_read_lock();
> >> +
> >> + list_for_each_entry(peer, &stats->peers, list) {
> >> + /* This doesn't account for multiple STA connected on different
> >> + * vifs. Unfortunately there's no way to derive that from the available
> >> + * information.
> >> + */
> >> + sta = ieee80211_find_sta_by_ifaddr(hw,
> >> + peer->peer_macaddr,
> >> + localaddr);
> >> + if (!sta) {
> >> + /* This tries to update multicast rates */
> >> + ieee80211_iterate_active_interfaces_atomic(
> >> + hw,
> >> + IEEE80211_IFACE_ITER_NORMAL,
> >> + ath10k_mac_update_txrx_rate_iter,
> >> + peer);
> >> + continue;
> >> + }
> >> +
> >> + arsta = (void *)sta->drv_priv;
> >> + arsta->tx_rate_kbps = peer->peer_tx_rate;
> >> + arsta->rx_rate_kbps = peer->peer_rx_rate;
> >> + }
> >> +
> >> + rcu_read_unlock();
> >> +}
> >> +
> >> void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
> >> {
> >> struct ath10k_fw_stats stats = {};
> >> @@ -335,6 +387,8 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
> >> goto free;
> >> }
> >>
> >> + ath10k_mac_update_txrx_rate(ar, &stats);
> >> +
> >> /* Stat data may exceed htc-wmi buffer limit. In such case firmware
> >> * splits the stats data and delivers it in a ping-pong fashion of
> >> * request cmd-update event.
> >> @@ -351,13 +405,6 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
> >> if (peer_stats_svc)
> >> ath10k_sta_update_rx_duration(ar, &stats.peers);
> >>
> >> - if (ar->debug.fw_stats_done) {
> >> - if (!peer_stats_svc)
> >> - ath10k_warn(ar, "received unsolicited stats update event\n");
> >> -
> >> - goto free;
> >> - }
> >> -
> >
> > [shafi] As you had suggested previously, should we completely clean up this ping
> > - pong response approach for f/w stats, (or) this should be retained to support
> > backward compatibility and also for supporting ping - pong response when user
> > cats for fw-stats (via debugfs) (i did see in the commit message this needs to
> > be cleaned up)
>
> I think it makes sense to remove the ping-pong logic and rely on
> periodic updates alone, including fw_stats and ethstats handling.
>
>
> >> - if (test_bit(WMI_SERVICE_PEER_STATS, ar->wmi.svc_map)) {
> >> - param = ar->wmi.pdev_param->peer_stats_update_period;
> >> - ret = ath10k_wmi_pdev_set_param(ar, param,
> >> - PEER_DEFAULT_STATS_UPDATE_PERIOD);
> >> - if (ret) {
> >> - ath10k_warn(ar,
> >> - "failed to set peer stats period : %d\n",
> >> - ret);
> >> - goto err_core_stop;
> >> - }
> >> + param = ar->wmi.pdev_param->peer_stats_update_period;
> >> + ret = ath10k_wmi_pdev_set_param(ar, param,
> >> + PEER_DEFAULT_STATS_UPDATE_PERIOD);
> >> + if (ret) {
> >> + ath10k_warn(ar,
> >> + "failed to set peer stats period : %d\n",
> >> + ret);
> >> + goto err_core_stop;
> >> }
> >
> > [shafi] If i am correct this change requires 'PEER_STATS' to be enabled by
> > default.
>
> No, it does not. Periodic stats have been available since forever.
[shafi] Michal, sorry i was talking about enabling WMI_PEER_STATS feature for
10.2, and we have a patch pushed recently to reduce the number of peers if
'WMI_PEER_STATS' feature is enabled(avoiding f/w crash due to memory
constraints) . But this patch requires the feature to be
turned ON always (with periodic stats update as well for evey 100ms). Please
correct me if my understanding is wrong.
>
>
> >> diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
> >> index 4d3cbc44fcd2..2877a3a27b95 100644
> >> --- a/drivers/net/wireless/ath/ath10k/wmi.h
> >> +++ b/drivers/net/wireless/ath/ath10k/wmi.h
> >> @@ -3296,7 +3296,7 @@ struct wmi_csa_event {
> >> /* the definition of different PDEV parameters */
> >> #define PDEV_DEFAULT_STATS_UPDATE_PERIOD 500
> >> #define VDEV_DEFAULT_STATS_UPDATE_PERIOD 500
> >> -#define PEER_DEFAULT_STATS_UPDATE_PERIOD 500
> >> +#define PEER_DEFAULT_STATS_UPDATE_PERIOD 100
> >
> > [shafi] Is this for more granularity since 500ms is not sufficient ?, I understand
> > the firmware has default stats_update_period as 500ms and i hope it supports
> > 100ms as well, Also if we are going to support period stats update we may need to
> > accumulate the information in driver (like this change and rx_duration
>
> The patch is used for rough rate estimation which is used to keep Tx
> queues filled only with 1-2 txops worth of data. Signal conditions can
> change vastly so I figured I need the peer stat update events come
> more often. I didn't really verify if they come every 100ms. The patch
> already served it's job as a proof-of-concept for smarter tx queuing.
[shafi] Ok, sure. I understand very soon we will get this in ath10k as well
(though it is a POC)
>
> > I will try to take this change, rebase it to TOT and see how it goes.
>
> There's really no benefit it taking this patch as a basis for periodic
> stat handling. Majority of this patch is just handling peer_tx_rate
> for rate estimation purposes. Feel free to knock yourself out with
> ripping out the ping-pong stat stuff though :)
>
[shafi] sure Michal, thanks !
regards,
shafi
^ permalink raw reply
* [PATCH net 7/7] net: hns: bug fix about getting hilink status for HNS v2
From: Yisen Zhuang @ 2016-03-24 11:08 UTC (permalink / raw)
To: davem
Cc: yankejian, huangdaode, salil.mehta, lisheng011, lipeng321,
liguozhu, xieqianqian, netdev, linux-kernel, linuxarm
In-Reply-To: <1458817686-159708-1-git-send-email-Yisen.Zhuang@huawei.com>
From: Sheng Li <lisheng011@huawei.com>
The hilink status reg in HNS V2 is different from HNS v1. In HNS V2, It
distinguishes differnt lane status according to the bit-field of the reg.
As is shown below:
[0:0] ---> lane0
[1:1] ---> lane1
...
But the current driver reads the reg to get the hilink status ONLY
concidering HNS V1 situation. Here is a patch to support both of them.
Signed-off-by: Sheng Li <lisheng011@huawei.com>
Signed-off-by: Daode Huang <huangdaode@hisilicon.com>
---
drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c | 40 ++++++++++++----------
drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h | 2 ++
2 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c
index 607c3be..e69b022 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c
@@ -244,31 +244,35 @@ void hns_ppe_com_srst(struct ppe_common_cb *ppe_common, u32 val)
*/
phy_interface_t hns_mac_get_phy_if(struct hns_mac_cb *mac_cb)
{
- u32 hilink3_mode;
- u32 hilink4_mode;
+ u32 mode;
+ u32 reg;
+ u32 shift;
+ bool is_ver1 = AE_IS_VER1(mac_cb->dsaf_dev->dsaf_ver);
void __iomem *sys_ctl_vaddr = mac_cb->sys_ctl_vaddr;
- int dev_id = mac_cb->mac_id;
+ int mac_id = mac_cb->mac_id;
phy_interface_t phy_if = PHY_INTERFACE_MODE_NA;
- hilink3_mode = dsaf_read_reg(sys_ctl_vaddr, HNS_MAC_HILINK3_REG);
- hilink4_mode = dsaf_read_reg(sys_ctl_vaddr, HNS_MAC_HILINK4_REG);
- if (dev_id >= 0 && dev_id <= 3) {
- if (hilink4_mode == 0)
- phy_if = PHY_INTERFACE_MODE_SGMII;
- else
+ if (is_ver1 && (mac_id >= 6 && mac_id <= 7)) {
+ phy_if = PHY_INTERFACE_MODE_SGMII;
+ } else if (mac_id >= 0 && mac_id <= 3) {
+ reg = is_ver1 ? HNS_MAC_HILINK4_REG : HNS_MAC_HILINK4V2_REG;
+ mode = dsaf_read_reg(sys_ctl_vaddr, reg);
+ /* mac_id 0, 1, 2, 3 ---> hilink4 lane 0, 1, 2, 3 */
+ shift = is_ver1 ? 0 : mac_id;
+ if (dsaf_get_bit(mode, shift))
phy_if = PHY_INTERFACE_MODE_XGMII;
- } else if (dev_id >= 4 && dev_id <= 5) {
- if (hilink3_mode == 0)
- phy_if = PHY_INTERFACE_MODE_SGMII;
else
+ phy_if = PHY_INTERFACE_MODE_SGMII;
+ } else if (mac_id >= 4 && mac_id <= 7) {
+ reg = is_ver1 ? HNS_MAC_HILINK3_REG : HNS_MAC_HILINK3V2_REG;
+ mode = dsaf_read_reg(sys_ctl_vaddr, reg);
+ /* mac_id 4, 5, 6, 7 ---> hilink3 lane 2, 3, 0, 1 */
+ shift = is_ver1 ? 0 : mac_id <= 5 ? mac_id - 2 : mac_id - 6;
+ if (dsaf_get_bit(mode, shift))
phy_if = PHY_INTERFACE_MODE_XGMII;
- } else {
- phy_if = PHY_INTERFACE_MODE_SGMII;
+ else
+ phy_if = PHY_INTERFACE_MODE_SGMII;
}
-
- dev_dbg(mac_cb->dev,
- "hilink3_mode=%d, hilink4_mode=%d dev_id=%d, phy_if=%d\n",
- hilink3_mode, hilink4_mode, dev_id, phy_if);
return phy_if;
}
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h
index bf62687..e2206f9 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h
@@ -103,6 +103,8 @@
/*serdes offset**/
#define HNS_MAC_HILINK3_REG DSAF_SUB_SC_HILINK3_CRG_CTRL0_REG
#define HNS_MAC_HILINK4_REG DSAF_SUB_SC_HILINK4_CRG_CTRL0_REG
+#define HNS_MAC_HILINK3V2_REG DSAF_SUB_SC_HILINK3_CRG_CTRL1_REG
+#define HNS_MAC_HILINK4V2_REG DSAF_SUB_SC_HILINK4_CRG_CTRL1_REG
#define HNS_MAC_LANE0_CTLEDFE_REG 0x000BFFCCULL
#define HNS_MAC_LANE1_CTLEDFE_REG 0x000BFFBCULL
#define HNS_MAC_LANE2_CTLEDFE_REG 0x000BFFACULL
--
1.9.1
^ permalink raw reply related
* [PATCH net 6/7] net: hns: fix warning of passing zero to 'PTR_ERR'
From: Yisen Zhuang @ 2016-03-24 11:08 UTC (permalink / raw)
To: davem
Cc: yankejian, huangdaode, salil.mehta, lisheng011, lipeng321,
liguozhu, xieqianqian, netdev, linux-kernel, linuxarm
In-Reply-To: <1458817686-159708-1-git-send-email-Yisen.Zhuang@huawei.com>
From: Kejian Yan <yankejian@huawei.com>
There is a misuse of PTR as shown below:
ae_node = (void *)of_parse_phandle(dev->of_node,
"ae-handle",
0);
if (IS_ERR_OR_NULL(ae_node)) {
ret = PTR_ERR(ae_node);
dev_err(dev, "not find ae-handle\n");
goto out_read_prop_fail;
}
if the ae_node is NULL, PTR_ERR(ae_node) means it returns success. And the
return value should be -ENODEV.
Signed-off-by: Kejian Yan <yankejian@huawei.com>
Signed-off-by: Yisen Zhuang <Yisen.Zhuang@huawei.com>
---
drivers/net/ethernet/hisilicon/hns/hns_enet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index 66d1652..687204b 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -1814,7 +1814,7 @@ static int hns_nic_try_get_ae(struct net_device *ndev)
h = hnae_get_handle(&priv->netdev->dev,
priv->ae_node, priv->port_id, NULL);
if (IS_ERR_OR_NULL(h)) {
- ret = PTR_ERR(h);
+ ret = -ENODEV;
dev_dbg(priv->dev, "has not handle, register notifier!\n");
goto out;
}
--
1.9.1
^ permalink raw reply related
* [PATCH net 5/7] net: hns: optimizate irq proccess for HNS V2
From: Yisen Zhuang @ 2016-03-24 11:08 UTC (permalink / raw)
To: davem
Cc: yankejian, huangdaode, salil.mehta, lisheng011, lipeng321,
liguozhu, xieqianqian, netdev, linux-kernel, linuxarm
In-Reply-To: <1458817686-159708-1-git-send-email-Yisen.Zhuang@huawei.com>
From: Sheng Li <lisheng011@huawei.com>
In hns V1, common_poll should check and clean fbd pkts, because it
can not pend irq to clean them if there is no new pkt comes in.
But hns V2 hw fixes this bug, and will pend irq itself to do this.
So, for hns V2, we set ring_data->fini_process to NULL.
Signed-off-by: Sheng Li <lisheng011@huawei.com>
Signed-off-by: Yisen Zhuang <Yisen.Zhuang@huawei.com>
---
drivers/net/ethernet/hisilicon/hns/hns_enet.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index f0c9a41..66d1652 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -956,8 +956,8 @@ static int hns_nic_common_poll(struct napi_struct *napi, int budget)
napi_complete(napi);
ring_data->ring->q->handle->dev->ops->toggle_ring_irq(
ring_data->ring, 0);
-
- ring_data->fini_process(ring_data);
+ if (ring_data->fini_process)
+ ring_data->fini_process(ring_data);
return 0;
}
@@ -1720,6 +1720,7 @@ static int hns_nic_init_ring_data(struct hns_nic_priv *priv)
{
struct hnae_handle *h = priv->ae_handle;
struct hns_nic_ring_data *rd;
+ bool is_ver1 = AE_IS_VER1(priv->enet_ver);
int i;
if (h->q_num > NIC_MAX_Q_PER_VF) {
@@ -1737,7 +1738,7 @@ static int hns_nic_init_ring_data(struct hns_nic_priv *priv)
rd->queue_index = i;
rd->ring = &h->qs[i]->tx_ring;
rd->poll_one = hns_nic_tx_poll_one;
- rd->fini_process = hns_nic_tx_fini_pro;
+ rd->fini_process = is_ver1 ? hns_nic_tx_fini_pro : NULL;
netif_napi_add(priv->netdev, &rd->napi,
hns_nic_common_poll, NIC_TX_CLEAN_MAX_NUM);
@@ -1749,7 +1750,7 @@ static int hns_nic_init_ring_data(struct hns_nic_priv *priv)
rd->ring = &h->qs[i - h->q_num]->rx_ring;
rd->poll_one = hns_nic_rx_poll_one;
rd->ex_process = hns_nic_rx_up_pro;
- rd->fini_process = hns_nic_rx_fini_pro;
+ rd->fini_process = is_ver1 ? hns_nic_rx_fini_pro : NULL;
netif_napi_add(priv->netdev, &rd->napi,
hns_nic_common_poll, NIC_RX_CLEAN_MAX_NUM);
--
1.9.1
^ permalink raw reply related
* [PATCH net 1/7] net: hns: fix a bug for cycle index
From: Yisen Zhuang @ 2016-03-24 11:08 UTC (permalink / raw)
To: davem
Cc: yankejian, huangdaode, salil.mehta, lisheng011, lipeng321,
liguozhu, xieqianqian, netdev, linux-kernel, linuxarm
In-Reply-To: <1458817686-159708-1-git-send-email-Yisen.Zhuang@huawei.com>
From: Qianqian Xie <xieqianqian@huawei.com>
The cycle index should be varied while the variable j is a fixed value.
The patch will fix this bug.
Signed-off-by: Qianqian Xie <xieqianqian@huawei.com>
Signed-off-by: Yisen Zhuang <Yisen.Zhuang@huawei.com>
---
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
index 5c1ac9b..5978a5c 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
@@ -2219,17 +2219,17 @@ void hns_dsaf_get_regs(struct dsaf_device *ddev, u32 port, void *data)
/* dsaf onode registers */
for (i = 0; i < DSAF_XOD_NUM; i++) {
p[311 + i] = dsaf_read_dev(ddev,
- DSAF_XOD_ETS_TSA_TC0_TC3_CFG_0_REG + j * 0x90);
+ DSAF_XOD_ETS_TSA_TC0_TC3_CFG_0_REG + i * 0x90);
p[319 + i] = dsaf_read_dev(ddev,
- DSAF_XOD_ETS_TSA_TC4_TC7_CFG_0_REG + j * 0x90);
+ DSAF_XOD_ETS_TSA_TC4_TC7_CFG_0_REG + i * 0x90);
p[327 + i] = dsaf_read_dev(ddev,
- DSAF_XOD_ETS_BW_TC0_TC3_CFG_0_REG + j * 0x90);
+ DSAF_XOD_ETS_BW_TC0_TC3_CFG_0_REG + i * 0x90);
p[335 + i] = dsaf_read_dev(ddev,
- DSAF_XOD_ETS_BW_TC4_TC7_CFG_0_REG + j * 0x90);
+ DSAF_XOD_ETS_BW_TC4_TC7_CFG_0_REG + i * 0x90);
p[343 + i] = dsaf_read_dev(ddev,
- DSAF_XOD_ETS_BW_OFFSET_CFG_0_REG + j * 0x90);
+ DSAF_XOD_ETS_BW_OFFSET_CFG_0_REG + i * 0x90);
p[351 + i] = dsaf_read_dev(ddev,
- DSAF_XOD_ETS_TOKEN_CFG_0_REG + j * 0x90);
+ DSAF_XOD_ETS_TOKEN_CFG_0_REG + i * 0x90);
}
p[359] = dsaf_read_dev(ddev, DSAF_XOD_PFS_CFG_0_0_REG + port * 0x90);
--
1.9.1
^ permalink raw reply related
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