* [PATCH 2/7] netfilter: Convert print_tuple functions to return void
From: Joe Perches @ 2014-09-29 23:08 UTC (permalink / raw)
To: Steven Rostedt
Cc: Al Viro, Petr Mladek, Andrew Morton, Linus Torvalds,
Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
David S. Miller, netfilter-devel, coreteam, netdev, linux-kernel
In-Reply-To: <cover.1412031505.git.joe@perches.com>
Since adding a new function to seq_file (seq_is_full)
there isn't any value for functions called from seq_show to
return anything. Remove the int returns of the various
print_tuple/<foo>_print_tuple functions.
Signed-off-by: Joe Perches <joe@perches.com>
---
include/net/netfilter/nf_conntrack_core.h | 2 +-
include/net/netfilter/nf_conntrack_l3proto.h | 4 ++--
include/net/netfilter/nf_conntrack_l4proto.h | 4 ++--
net/netfilter/nf_conntrack_l3proto_generic.c | 5 ++---
net/netfilter/nf_conntrack_proto_dccp.c | 10 +++++-----
net/netfilter/nf_conntrack_proto_generic.c | 5 ++---
net/netfilter/nf_conntrack_proto_gre.c | 10 +++++-----
net/netfilter/nf_conntrack_proto_sctp.c | 10 +++++-----
net/netfilter/nf_conntrack_proto_tcp.c | 10 +++++-----
net/netfilter/nf_conntrack_proto_udp.c | 10 +++++-----
net/netfilter/nf_conntrack_proto_udplite.c | 10 +++++-----
net/netfilter/nf_conntrack_standalone.c | 15 +++++++--------
12 files changed, 46 insertions(+), 49 deletions(-)
diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
index cc0c188..f2f0fa3 100644
--- a/include/net/netfilter/nf_conntrack_core.h
+++ b/include/net/netfilter/nf_conntrack_core.h
@@ -72,7 +72,7 @@ static inline int nf_conntrack_confirm(struct sk_buff *skb)
return ret;
}
-int
+void
print_tuple(struct seq_file *s, const struct nf_conntrack_tuple *tuple,
const struct nf_conntrack_l3proto *l3proto,
const struct nf_conntrack_l4proto *proto);
diff --git a/include/net/netfilter/nf_conntrack_l3proto.h b/include/net/netfilter/nf_conntrack_l3proto.h
index adc1fa3..cdc920b 100644
--- a/include/net/netfilter/nf_conntrack_l3proto.h
+++ b/include/net/netfilter/nf_conntrack_l3proto.h
@@ -38,8 +38,8 @@ struct nf_conntrack_l3proto {
const struct nf_conntrack_tuple *orig);
/* Print out the per-protocol part of the tuple. */
- int (*print_tuple)(struct seq_file *s,
- const struct nf_conntrack_tuple *);
+ void (*print_tuple)(struct seq_file *s,
+ const struct nf_conntrack_tuple *);
/*
* Called before tracking.
diff --git a/include/net/netfilter/nf_conntrack_l4proto.h b/include/net/netfilter/nf_conntrack_l4proto.h
index 4c8d573..fead8ee 100644
--- a/include/net/netfilter/nf_conntrack_l4proto.h
+++ b/include/net/netfilter/nf_conntrack_l4proto.h
@@ -56,8 +56,8 @@ struct nf_conntrack_l4proto {
u_int8_t pf, unsigned int hooknum);
/* Print out the per-protocol part of the tuple. Return like seq_* */
- int (*print_tuple)(struct seq_file *s,
- const struct nf_conntrack_tuple *);
+ void (*print_tuple)(struct seq_file *s,
+ const struct nf_conntrack_tuple *);
/* Print out the private part of the conntrack. */
int (*print_conntrack)(struct seq_file *s, struct nf_conn *);
diff --git a/net/netfilter/nf_conntrack_l3proto_generic.c b/net/netfilter/nf_conntrack_l3proto_generic.c
index e7eb807..cf9ace7 100644
--- a/net/netfilter/nf_conntrack_l3proto_generic.c
+++ b/net/netfilter/nf_conntrack_l3proto_generic.c
@@ -49,10 +49,9 @@ static bool generic_invert_tuple(struct nf_conntrack_tuple *tuple,
return true;
}
-static int generic_print_tuple(struct seq_file *s,
- const struct nf_conntrack_tuple *tuple)
+static void generic_print_tuple(struct seq_file *s,
+ const struct nf_conntrack_tuple *tuple)
{
- return 0;
}
static int generic_get_l4proto(const struct sk_buff *skb, unsigned int nhoff,
diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
index cb372f9..40d96f9 100644
--- a/net/netfilter/nf_conntrack_proto_dccp.c
+++ b/net/netfilter/nf_conntrack_proto_dccp.c
@@ -618,12 +618,12 @@ out_invalid:
return -NF_ACCEPT;
}
-static int dccp_print_tuple(struct seq_file *s,
- const struct nf_conntrack_tuple *tuple)
+static void dccp_print_tuple(struct seq_file *s,
+ const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "sport=%hu dport=%hu ",
- ntohs(tuple->src.u.dccp.port),
- ntohs(tuple->dst.u.dccp.port));
+ seq_printf(s, "sport=%hu dport=%hu ",
+ ntohs(tuple->src.u.dccp.port),
+ ntohs(tuple->dst.u.dccp.port));
}
static int dccp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
diff --git a/net/netfilter/nf_conntrack_proto_generic.c b/net/netfilter/nf_conntrack_proto_generic.c
index d25f293..0a3ded1 100644
--- a/net/netfilter/nf_conntrack_proto_generic.c
+++ b/net/netfilter/nf_conntrack_proto_generic.c
@@ -39,10 +39,9 @@ static bool generic_invert_tuple(struct nf_conntrack_tuple *tuple,
}
/* Print out the per-protocol part of the tuple. */
-static int generic_print_tuple(struct seq_file *s,
- const struct nf_conntrack_tuple *tuple)
+static void generic_print_tuple(struct seq_file *s,
+ const struct nf_conntrack_tuple *tuple)
{
- return 0;
}
static unsigned int *generic_get_timeouts(struct net *net)
diff --git a/net/netfilter/nf_conntrack_proto_gre.c b/net/netfilter/nf_conntrack_proto_gre.c
index d566573..cd85336 100644
--- a/net/netfilter/nf_conntrack_proto_gre.c
+++ b/net/netfilter/nf_conntrack_proto_gre.c
@@ -226,12 +226,12 @@ static bool gre_pkt_to_tuple(const struct sk_buff *skb, unsigned int dataoff,
}
/* print gre part of tuple */
-static int gre_print_tuple(struct seq_file *s,
- const struct nf_conntrack_tuple *tuple)
+static void gre_print_tuple(struct seq_file *s,
+ const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "srckey=0x%x dstkey=0x%x ",
- ntohs(tuple->src.u.gre.key),
- ntohs(tuple->dst.u.gre.key));
+ seq_printf(s, "srckey=0x%x dstkey=0x%x ",
+ ntohs(tuple->src.u.gre.key),
+ ntohs(tuple->dst.u.gre.key));
}
/* print private data for conntrack */
diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
index 1314d33..1b1d0e9 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -166,12 +166,12 @@ static bool sctp_invert_tuple(struct nf_conntrack_tuple *tuple,
}
/* Print out the per-protocol part of the tuple. */
-static int sctp_print_tuple(struct seq_file *s,
- const struct nf_conntrack_tuple *tuple)
+static void sctp_print_tuple(struct seq_file *s,
+ const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "sport=%hu dport=%hu ",
- ntohs(tuple->src.u.sctp.port),
- ntohs(tuple->dst.u.sctp.port));
+ seq_printf(s, "sport=%hu dport=%hu ",
+ ntohs(tuple->src.u.sctp.port),
+ ntohs(tuple->dst.u.sctp.port));
}
/* Print out the private part of the conntrack. */
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index 44d1ea3..c2693e78 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -302,12 +302,12 @@ static bool tcp_invert_tuple(struct nf_conntrack_tuple *tuple,
}
/* Print out the per-protocol part of the tuple. */
-static int tcp_print_tuple(struct seq_file *s,
- const struct nf_conntrack_tuple *tuple)
+static void tcp_print_tuple(struct seq_file *s,
+ const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "sport=%hu dport=%hu ",
- ntohs(tuple->src.u.tcp.port),
- ntohs(tuple->dst.u.tcp.port));
+ seq_printf(s, "sport=%hu dport=%hu ",
+ ntohs(tuple->src.u.tcp.port),
+ ntohs(tuple->dst.u.tcp.port));
}
/* Print out the private part of the conntrack. */
diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
index 9d7721c..6957281 100644
--- a/net/netfilter/nf_conntrack_proto_udp.c
+++ b/net/netfilter/nf_conntrack_proto_udp.c
@@ -63,12 +63,12 @@ static bool udp_invert_tuple(struct nf_conntrack_tuple *tuple,
}
/* Print out the per-protocol part of the tuple. */
-static int udp_print_tuple(struct seq_file *s,
- const struct nf_conntrack_tuple *tuple)
+static void udp_print_tuple(struct seq_file *s,
+ const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "sport=%hu dport=%hu ",
- ntohs(tuple->src.u.udp.port),
- ntohs(tuple->dst.u.udp.port));
+ seq_printf(s, "sport=%hu dport=%hu ",
+ ntohs(tuple->src.u.udp.port),
+ ntohs(tuple->dst.u.udp.port));
}
static unsigned int *udp_get_timeouts(struct net *net)
diff --git a/net/netfilter/nf_conntrack_proto_udplite.c b/net/netfilter/nf_conntrack_proto_udplite.c
index 2750e6c..c5903d1 100644
--- a/net/netfilter/nf_conntrack_proto_udplite.c
+++ b/net/netfilter/nf_conntrack_proto_udplite.c
@@ -71,12 +71,12 @@ static bool udplite_invert_tuple(struct nf_conntrack_tuple *tuple,
}
/* Print out the per-protocol part of the tuple. */
-static int udplite_print_tuple(struct seq_file *s,
- const struct nf_conntrack_tuple *tuple)
+static void udplite_print_tuple(struct seq_file *s,
+ const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "sport=%hu dport=%hu ",
- ntohs(tuple->src.u.udp.port),
- ntohs(tuple->dst.u.udp.port));
+ seq_printf(s, "sport=%hu dport=%hu ",
+ ntohs(tuple->src.u.udp.port),
+ ntohs(tuple->dst.u.udp.port));
}
static unsigned int *udplite_get_timeouts(struct net *net)
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index cf65a1e..3c2ce5c 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -36,12 +36,13 @@
MODULE_LICENSE("GPL");
#ifdef CONFIG_NF_CONNTRACK_PROCFS
-int
+void
print_tuple(struct seq_file *s, const struct nf_conntrack_tuple *tuple,
const struct nf_conntrack_l3proto *l3proto,
const struct nf_conntrack_l4proto *l4proto)
{
- return l3proto->print_tuple(s, tuple) || l4proto->print_tuple(s, tuple);
+ l3proto->print_tuple(s, tuple);
+ l4proto->print_tuple(s, tuple);
}
EXPORT_SYMBOL_GPL(print_tuple);
@@ -202,9 +203,8 @@ static int ct_seq_show(struct seq_file *s, void *v)
if (l4proto->print_conntrack && l4proto->print_conntrack(s, ct))
goto release;
- if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
- l3proto, l4proto))
- goto release;
+ print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
+ l3proto, l4proto);
if (seq_print_acct(s, ct, IP_CT_DIR_ORIGINAL))
goto release;
@@ -213,9 +213,8 @@ static int ct_seq_show(struct seq_file *s, void *v)
if (seq_printf(s, "[UNREPLIED] "))
goto release;
- if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
- l3proto, l4proto))
- goto release;
+ print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
+ l3proto, l4proto);
if (seq_print_acct(s, ct, IP_CT_DIR_REPLY))
goto release;
--
1.8.1.2.459.gbcd45b4.dirty
^ permalink raw reply related
* [PATCH 0/7] seq_printf cleanups
From: Joe Perches @ 2014-09-29 23:08 UTC (permalink / raw)
To: Steven Rostedt
Cc: Al Viro, Petr Mladek, Andrew Morton, Linus Torvalds, linux-doc,
linux-kernel, linux-mtd, netdev, cluster-devel, linux-fsdevel,
netfilter-devel, coreteam
In-Reply-To: <20140929124246.3e39dac8@gandalf.local.home>
seq_printf should return void.
Add a public bool seq_is_full function that can be used to shortcut
unnecesary seq_printf/seq_puts calls when the seq buffer is full.
Start removing the misuses of the seq_printf/seq_puts return value.
Patchset brought forward from an unreplied to set of changes from
back in December 2013.
https://lkml.org/lkml/2013/12/11/713
Renamed seq_is_buf_full to seq_is_full.
Joe Perches (7):
seq_file: Rename static bool seq_overflow to public bool seq_is_full
netfilter: Convert print_tuple functions to return void
dlm: Use seq_is_full - remove seq_printf returns
dlm: Use seq_puts, remove unnecessary trailing spaces
fs: Convert show_fdinfo functions to void
debugfs: Fix misuse of seq_printf return value
docg3: Fix mixuse of seq_printf return value
Documentation/filesystems/seq_file.txt | 28 +--
Documentation/filesystems/vfs.txt | 2 +-
drivers/mtd/devices/docg3.c | 112 ++++++------
drivers/net/tun.c | 4 +-
fs/debugfs/file.c | 14 +-
fs/dlm/debug_fs.c | 260 +++++++++++++--------------
fs/eventfd.c | 15 +-
fs/eventpoll.c | 19 +-
fs/notify/fdinfo.c | 76 ++++----
fs/notify/fdinfo.h | 4 +-
fs/proc/fd.c | 2 +-
fs/seq_file.c | 28 +--
fs/signalfd.c | 10 +-
fs/timerfd.c | 27 +--
include/linux/fs.h | 2 +-
include/linux/seq_file.h | 8 +
include/net/netfilter/nf_conntrack_core.h | 2 +-
include/net/netfilter/nf_conntrack_l3proto.h | 4 +-
include/net/netfilter/nf_conntrack_l4proto.h | 4 +-
net/netfilter/nf_conntrack_l3proto_generic.c | 5 +-
net/netfilter/nf_conntrack_proto_dccp.c | 10 +-
net/netfilter/nf_conntrack_proto_generic.c | 5 +-
net/netfilter/nf_conntrack_proto_gre.c | 10 +-
net/netfilter/nf_conntrack_proto_sctp.c | 10 +-
net/netfilter/nf_conntrack_proto_tcp.c | 10 +-
net/netfilter/nf_conntrack_proto_udp.c | 10 +-
net/netfilter/nf_conntrack_proto_udplite.c | 10 +-
net/netfilter/nf_conntrack_standalone.c | 15 +-
28 files changed, 333 insertions(+), 373 deletions(-)
--
1.8.1.2.459.gbcd45b4.dirty
^ permalink raw reply
* [PATCH iproute2] tests: Dont fail cls-testbed.t if tests/cls dir is empty
From: Vadim Kochan @ 2014-09-29 22:57 UTC (permalink / raw)
To: netdev; +Cc: Vadim Kochan
Curently tests/cls-testbed.t tries to run any *.t in
tests/cls/ folder but such folder does not exist.
The better solution can be like having a generic mechanism which
allows to SKIP some tests (by return code?).
Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
testsuite/tests/cls-testbed.t | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/testsuite/tests/cls-testbed.t b/testsuite/tests/cls-testbed.t
index efae2a5..be32d79 100755
--- a/testsuite/tests/cls-testbed.t
+++ b/testsuite/tests/cls-testbed.t
@@ -5,6 +5,11 @@ source lib/generic.sh
QDISCS="cbq htb dsmark"
+if [ ! -d tests/cls ]; then
+ ts_log "tests/cls folder is empty"
+ exit 0
+fi
+
for q in ${QDISCS}; do
ts_log "Preparing classifier testbed with qdisc $q"
--
2.1.0
^ permalink raw reply related
* [PATCH iproute2] tests: Allow policer test to be ran
From: Vadim Kochan @ 2014-09-29 22:22 UTC (permalink / raw)
To: netdev; +Cc: Vadim Kochan
Renamed testsuite/tests/policer to testsuite/tests/policer.t
Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
testsuite/tests/policer | 13 -------------
testsuite/tests/policer.t | 13 +++++++++++++
2 files changed, 13 insertions(+), 13 deletions(-)
delete mode 100755 testsuite/tests/policer
create mode 100755 testsuite/tests/policer.t
diff --git a/testsuite/tests/policer b/testsuite/tests/policer
deleted file mode 100755
index eaf16ac..0000000
--- a/testsuite/tests/policer
+++ /dev/null
@@ -1,13 +0,0 @@
-#!/bin/sh
-$TC qdisc del dev $DEV root >/dev/null 2>&1
-$TC qdisc add dev $DEV root handle 10:0 cbq bandwidth 100Mbit avpkt 1400 mpu 64
-$TC class add dev $DEV parent 10:0 classid 10:12 cbq bandwidth 100mbit rate 100mbit allot 1514 prio 3 maxburst 1 avpkt 500 bounded
-$TC filter add dev $DEV parent 10:0 protocol ip prio 10 u32 match ip protocol 1 0xff police rate 2kbit buffer 10k drop flowid 10:12
-$TC qdisc list dev $DEV
-$TC filter list dev $DEV parent 10:0
-$TC qdisc del dev $DEV root
-$TC qdisc list dev $DEV
-$TC qdisc add dev $DEV root handle 10:0 cbq bandwidth 100Mbit avpkt 1400 mpu 64
-$TC class add dev $DEV parent 10:0 classid 10:12 cbq bandwidth 100mbit rate 100mbit allot 1514 prio 3 maxburst 1 avpkt 500 bounded
-$TC filter add dev $DEV parent 10:0 protocol ip prio 10 u32 match ip protocol 1 0xff police rate 2kbit buffer 10k drop flowid 10:12
-$TC qdisc del dev $DEV root
diff --git a/testsuite/tests/policer.t b/testsuite/tests/policer.t
new file mode 100755
index 0000000..eaf16ac
--- /dev/null
+++ b/testsuite/tests/policer.t
@@ -0,0 +1,13 @@
+#!/bin/sh
+$TC qdisc del dev $DEV root >/dev/null 2>&1
+$TC qdisc add dev $DEV root handle 10:0 cbq bandwidth 100Mbit avpkt 1400 mpu 64
+$TC class add dev $DEV parent 10:0 classid 10:12 cbq bandwidth 100mbit rate 100mbit allot 1514 prio 3 maxburst 1 avpkt 500 bounded
+$TC filter add dev $DEV parent 10:0 protocol ip prio 10 u32 match ip protocol 1 0xff police rate 2kbit buffer 10k drop flowid 10:12
+$TC qdisc list dev $DEV
+$TC filter list dev $DEV parent 10:0
+$TC qdisc del dev $DEV root
+$TC qdisc list dev $DEV
+$TC qdisc add dev $DEV root handle 10:0 cbq bandwidth 100Mbit avpkt 1400 mpu 64
+$TC class add dev $DEV parent 10:0 classid 10:12 cbq bandwidth 100mbit rate 100mbit allot 1514 prio 3 maxburst 1 avpkt 500 bounded
+$TC filter add dev $DEV parent 10:0 protocol ip prio 10 u32 match ip protocol 1 0xff police rate 2kbit buffer 10k drop flowid 10:12
+$TC qdisc del dev $DEV root
--
2.1.0
^ permalink raw reply related
* Re: [PATCH v1 5/5] driver-core: add driver asynchronous probe support
From: Greg KH @ 2014-09-29 22:24 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Tejun Heo, Dmitry Torokhov, Takashi Iwai, Arjan van de Ven,
Tom Gundersen, rmilasan, werner, Oleg Nesterov, hare,
Benjamin Poirier, Santosh Rastapur, Petr Mladek, dbueso,
linux-kernel@vger.kernel.org, Tetsuo Handa, Joseph Salisbury,
Kay Sievers, One Thousand Gnomes, Tim Gardner, Pierre Fersing,
Andrew Morton, Nagalakshmi Nandigama, Praveen Krishnamoorthy
In-Reply-To: <CAB=NE6XKeoDnnBxupLSAXz9w3TXVgAW+Yw3o8-ugzzUgH7j13A@mail.gmail.com>
On Mon, Sep 29, 2014 at 03:10:22PM -0700, Luis R. Rodriguez wrote:
> On Mon, Sep 29, 2014 at 2:59 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> Sure make sense, I wasn't quite sure how to make this quite clear,
> >> a naming convention seems good to me but I also had added at least
> >> a print about this on the log. Ideally I think a TAIN_DEBUG would
> >> be best and it seems it could be useful for many other cases in
> >> the kernel, we could also just re-use TAINT_CRAP as well. Thoughts?
> >> Greg?
> >
> > TAINT_CRAP is for drivers/staging/ code, don't try to repurpose it for
> > some other horrid option. There's no reason we can't add more taint
> > flags for this.
>
> OK thanks, I'll add TAINT_DEBUG. Any preference where to stuff struct
> driver_attach_work *attach_work ? On the private data structure as
> this patch currently implements, saving us 24 bytes and hiding it from
> drivers, or stuffing it on the device driver and simplifying the core
> code?
I honestly haven't even looked at this series, sorry. It's too late
near the close of the merge window for 3.18 and have been on the road
for the past week in France.
greg k-h
^ permalink raw reply
* Re: [PATCH v1 5/5] driver-core: add driver asynchronous probe support
From: Luis R. Rodriguez @ 2014-09-29 22:10 UTC (permalink / raw)
To: Greg KH
Cc: Tejun Heo, Dmitry Torokhov, Takashi Iwai, Arjan van de Ven,
Tom Gundersen, rmilasan, werner, Oleg Nesterov, hare,
Benjamin Poirier, Santosh Rastapur, Petr Mladek, dbueso,
linux-kernel@vger.kernel.org, Tetsuo Handa, Joseph Salisbury,
Kay Sievers, One Thousand Gnomes, Tim Gardner, Pierre Fersing,
Andrew Morton, Nagalakshmi Nandigama, Praveen Krishnamoorthy
In-Reply-To: <20140929215945.GA1628@kroah.com>
On Mon, Sep 29, 2014 at 2:59 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> Sure make sense, I wasn't quite sure how to make this quite clear,
>> a naming convention seems good to me but I also had added at least
>> a print about this on the log. Ideally I think a TAIN_DEBUG would
>> be best and it seems it could be useful for many other cases in
>> the kernel, we could also just re-use TAINT_CRAP as well. Thoughts?
>> Greg?
>
> TAINT_CRAP is for drivers/staging/ code, don't try to repurpose it for
> some other horrid option. There's no reason we can't add more taint
> flags for this.
OK thanks, I'll add TAINT_DEBUG. Any preference where to stuff struct
driver_attach_work *attach_work ? On the private data structure as
this patch currently implements, saving us 24 bytes and hiding it from
drivers, or stuffing it on the device driver and simplifying the core
code?
Luis
^ permalink raw reply
* Re: [PATCH 32/34] netfilter: bridge: move br_netfilter out of the core
From: Eric Dumazet @ 2014-09-29 22:04 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, davem, netdev
In-Reply-To: <1411994363-8451-33-git-send-email-pablo@netfilter.org>
On Mon, 2014-09-29 at 14:39 +0200, Pablo Neira Ayuso wrote:
> Jesper reported that br_netfilter always registers the hooks since
> this is part of the bridge core. This harms performance for people that
> don't need this.
>
> This patch modularizes br_netfilter so it can be rmmod'ed, thus,
> the hooks can be unregistered. I think the bridge netfilter should have
> been a separated module since the beginning, Patrick agreed on that.
>
> Note that this is breaking compatibility for users that expect that
> bridge netfilter is going to be available after explicitly 'modprobe
> bridge' or via automatic load through brctl.
>
> However, the damage can be easily undone by modprobing br_netfilter.
> The bridge core also spots a message to provide a clue to people that
> didn't notice that this has been deprecated.
>
> On top of that, the plan is that nftables will not rely on this software
> layer, but integrate the connection tracking into the bridge layer to
> enable stateful filtering and NAT, which is was bridge netfilter users
> seem to require.
>
> This patch still keeps the fake_dst_ops in the bridge core, since this
> is required by when the bridge port is initialized. So we can safely
> modprobe/rmmod br_netfilter anytime.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Acked-by: Florian Westphal <fw@strlen.de>
> ---
Hmm... What am I missing here ?
$ grep CONFIG_BRIDGE_NETFILTER .config
# CONFIG_BRIDGE_NETFILTER is not set
$ make net/bridge/br_nf_core.o
CHK include/config/kernel.release
CHK include/generated/uapi/linux/version.h
CHK include/generated/utsrelease.h
CALL scripts/checksyscalls.sh
CC [M] net/bridge/br_nf_core.o
net/bridge/br_nf_core.c:77:1: error: expected identifier or ‘(’ before ‘{’ token
net/bridge/br_nf_core.c:88:12: error: redefinition of ‘br_nf_core_init’
net/bridge/br_private.h:762:19: note: previous definition of ‘br_nf_core_init’ was here
net/bridge/br_nf_core.c:93:6: error: redefinition of ‘br_nf_core_fini’
net/bridge/br_private.h:763:20: note: previous definition of ‘br_nf_core_fini’ was here
make[1]: *** [net/bridge/br_nf_core.o] Error 1
make: *** [net/bridge/br_nf_core.o] Error 2
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] staging: r8821ae: Remove driver from staging
From: Greg KH @ 2014-09-29 22:04 UTC (permalink / raw)
To: Larry Finger; +Cc: devel, netdev
In-Reply-To: <5429A015.1000700@lwfinger.net>
On Mon, Sep 29, 2014 at 01:08:21PM -0500, Larry Finger wrote:
> On 09/29/2014 12:05 PM, Greg KH wrote:
> >On Sun, Sep 28, 2014 at 08:39:51PM -0500, Larry Finger wrote:
> >>A new version of this driver has been merged into the wireless-testing tree
> >>as commit 21e4b0726dc671c423e2dc9a85364716219c4502. It is both possible and
> >>desirable to delete the staging version to avoid build errors in linux-next.
> >>
> >>Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> >>---
> >>
> >>Greg,
> >>
> >>This removal patch was generated after commit 368c75b98bdfdfad54e7f165016819ef344e3587.
> >>It should apply to your tree unless further modifications have been made to
> >>the staging driver.
> >
> >Something is odd with this patch, it doesn't apply, even against that
> >commit id:
> >Applying: staging: r8821ae: Remove driver from staging
> >error: patch failed: drivers/staging/rtl8821ae/btcoexist/HalBtc8812a1Ant.c:1
> >error: drivers/staging/rtl8821ae/btcoexist/HalBtc8812a1Ant.c: patch does not apply
> >Patch failed at 0001 staging: r8821ae: Remove driver from staging
> >The copy of the patch that failed is found in:
> > /home/gregkh/linux/work/staging/.git/rebase-apply/patch
> >When you have resolved this problem, run "git am --continue".
> >If you prefer to skip this patch, run "git am --skip" instead.
> >To restore the original branch and stop patching, run "git am --abort".
> >
> >Can you refresh against the staging-next branch of staging.git and try it
> >again? Or I can just delete it "by hand" which is fine with me, and might be
> >easier fro you, just let me know.
>
> I was on the staging-next branch, but when I pulled after receiving this
> mail, I got a conflict due to some change in
> drivers/staging/rtl8821ae/pci.c. That one must have been in your local copy,
> but not yet pushed when I pulled.
>
> If you don't mind doing the delete there, the traffic on the Cc sites will
> be reduced. I'm sure you know what is needed, but just in case, delete the
> files in drivers/staging/rtl8821ae/ and the Makefile and Kconfig entries in
> drivers/staging/.
Great, I've done this now in the staging-next tree, it will be merged
for 3.18-rc1.
Thanks for doing this work.
greg k-h
^ permalink raw reply
* Re: [PATCH v1 5/5] driver-core: add driver asynchronous probe support
From: Greg KH @ 2014-09-29 21:59 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Tejun Heo, Luis R. Rodriguez, dmitry.torokhov, tiwai, arjan, teg,
rmilasan, werner, oleg, hare, bpoirier, santosh, pmladek, dbueso,
linux-kernel, Tetsuo Handa, Joseph Salisbury, Kay Sievers,
One Thousand Gnomes, Tim Gardner, Pierre Fersing, Andrew Morton,
Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
Abhijit Mahajan, Casey Leedom
In-Reply-To: <20140929212208.GV17349@wotan.suse.de>
On Mon, Sep 29, 2014 at 11:22:08PM +0200, Luis R. Rodriguez wrote:
> On Sun, Sep 28, 2014 at 11:03:29AM -0400, Tejun Heo wrote:
> > Hello,
> >
> > On Fri, Sep 26, 2014 at 02:57:17PM -0700, Luis R. Rodriguez wrote:
> > ...
> > > Systemd should consider enabling async probe on device drivers
> > > it loads through systemd-udev but probably does not want to
> > > enable it for modules loaded through systemd-modules-load
> > > (modules-load.d). At least on my booting enablign async probe
> > > for all modules fails to boot as such in order to make this
> >
> > Did you find out why boot failed with those modules?
>
> No, it seems this was early in boot and I haven't been able to capture the logs
> yet of the faults. More on this below.
>
> > > a bit more useful we whitelist a few buses where it should be
> > > at least in theory safe to try to enable async probe. This
> > > way even if systemd tried to ask to enable async probe for all
> > > its device drivers the kernel won't blindly do this. We also
> > > have the sync_probe flag which device drivers can themselves
> > > enable *iff* its known the device driver should never async
> > > probe.
> > >
> > > In order to help *test* things folks can use the bus.safe_mod_async_probe=1
> > > kernel parameter which will work as if userspace would have
> > > requested all modules to load with async probe. Daring folks can
> > > also use bus.force_mod_async_probe=1 which will enable asynch probe
> > > even on buses not tested in any way yet, if you use that though
> > > you're on your own.
> >
> > If those two knobs are meant for debugging, let's please make that
> > fact immediately evident. e.g. Make them ugly boot params like
> > "__DEVEL__driver_force_mod_async_probe". Devel/debug options ending
> > up becoming stable interface are really nasty.
>
> Sure make sense, I wasn't quite sure how to make this quite clear,
> a naming convention seems good to me but I also had added at least
> a print about this on the log. Ideally I think a TAIN_DEBUG would
> be best and it seems it could be useful for many other cases in
> the kernel, we could also just re-use TAINT_CRAP as well. Thoughts?
> Greg?
TAINT_CRAP is for drivers/staging/ code, don't try to repurpose it for
some other horrid option. There's no reason we can't add more taint
flags for this.
greg k-h
^ permalink raw reply
* Re: Silicom bypass driver promote from staging
From: Francois Romieu @ 2014-09-29 21:52 UTC (permalink / raw)
To: David Hendel
Cc: netdev@vger.kernel.org, arnd@arndb.de, Anna Lukin,
gregkh@linuxfoundation.org
In-Reply-To: <3BFAEADE799AF145974162DF00E013AF018F8C8A9F@exchange2010.silicom.local>
David Hendel <david@silicom.co.il> :
> Hi all,
> Sorry, for the late response, we had to go over all the code and update as requested and then test to verify, so here it is.
Your messages should look like:
http://marc.info/?l=linux-netdev&m=140720472303311&w=2
(especially the Subject: field)
[...]
> >Notwithstanding Stephen and Jeff's remarks, you should also:
> >- remove the (out-)commented code
> >- reconsider the use of gorilla class macros vs plain functions
> >- stop casting ioremap return into long, then later into void *. It's
> > void __iomem * and should stay so.
> >- use a consistent locking style and remove BP_SYNC_FLAG
> >- fix the 80 cols limit problem(s) (hardly surprizing after 5 levels of
> > nested "if")
> >- avoid redefining stuff from include/uapi/linux/mii.h
> > I may be wrong but BPCTLI_MII_CR_POWER_DOWN smells of BMCR_PDOWN and
> > BPCTLI_PHY_CONTROL, well, MII_BMCR ?
> >- avoid returning with spinlock held (read_reg, wdt_pulse)
> >- start explaining what did change between two submissions
>
> This is what we changed in updated patch:
> 1. Avoid redefining BIT and MII .
> 2. Fixing returning with spinlock held
> 3. Remove BP_SYNC_FLAG
> 4. Use void __iomem * instead of long long in ioremap return
/me greps mem_map
> + iounmap((void *)dev->mem_map);
> + iounmap((void *)(bpctl_dev_arr[i].mem_map));
> + (writel((value), (void *)(((a)->mem_map) + BPCTLI_##reg)))
> + readl((void *)((a)->mem_map) + BPCTLI_##reg))
> + (writel((value), (void *)(((a)->mem_map) + BP10G_##reg)))
> + readl((void *)((a)->mem_map) + BP10G_##reg))
> + (writel((value), (void *)(((a)->mem_map) + BP10GB_##reg)))
> + readl((void *)((a)->mem_map) + BP10GB_##reg))
Please remove the useless void * casts
[...]
> diff -uprN -X linux-3.17-rc1-vanilla/Documentation/dontdiff linux-3.17-rc1-vanilla/drivers/bypass/silicom/bp_ctl/bpctl_mod.c linux-3.17-rc1/drivers/bypass/silicom/bp_ctl/bpctl_mod.c
> --- linux-3.17-rc1-vanilla/drivers/bypass/silicom/bp_ctl/bpctl_mod.c 1970-01-01 02:00:00.000000000 +0200
> +++ linux-3.17-rc1/drivers/bypass/silicom/bp_ctl/bpctl_mod.c 2014-09-09 00:43:05.000000000 +0300
[...]
> +static struct bpctl_dev *lookup_port(struct bpctl_dev *dev)
> +{
> + struct bpctl_dev *p;
> + int n;
> +
> + for (n = 0, p = bpctl_dev_arr; n < device_num && p->pdev; n++, p++) {
> + if (p->bus == dev->bus
> + && p->slot == dev->slot
> + && p->func == (dev->func ^ 1))
Binary operators should end the line. They shouldn't start it.
> + return p;
> + }
> + return NULL;
> +}
[...]
> +static int get_dev_idx_bsf(int bus, int slot, int func)
> +{
> + int idx_dev = 0;
> +
> + for (idx_dev = 0;
> + ((bpctl_dev_arr[idx_dev].pdev != NULL) &&
> + (idx_dev < device_num)); idx_dev++) {
> + if ((bus == bpctl_dev_arr[idx_dev].bus)
> + && (slot == bpctl_dev_arr[idx_dev].slot)
> + && (func == bpctl_dev_arr[idx_dev].func))
> +
> + return idx_dev;
> + }
> + return BP_NOT_CAP;
snort *p = bpctl_dev_arr;
int i;
for (i = 0; i < device_num && p->pdev; i++, p++) {
if (bus == p->bus && slot == p->slot && func == p-> func)
return i;
}
It appears multiple times.
> +}
> +
> +static int bp_get_dev_idx_bsf(struct net_device *dev, int *index)
> +{
> + struct ethtool_drvinfo drvinfo = {0};
> + char *buf;
> + int bus, slot, func;
> +
> + if (dev->ethtool_ops && dev->ethtool_ops->get_drvinfo)
> + dev->ethtool_ops->get_drvinfo(dev, &drvinfo);
> + else
> + return -EOPNOTSUPP;
if (!dev->ethtool_ops || !dev->ethtool_ops->get_drvinfo)
return -EOPNOTSUPP;
[...]
> +static void write_pulse(struct bpctl_dev *pbpctl_dev,
> + unsigned int ctrl_ext,
> + unsigned char value, unsigned char len)
> +{
> + unsigned char ctrl_val = 0;
> + unsigned int i = len;
> + unsigned int ctrl = 0;
> + struct bpctl_dev *pbpctl_dev_c = NULL;
> +
> + switch (pbpctl_dev->nic_type) {
> + case bp_i80:
> + ctrl = BPCTL_READ_REG(pbpctl_dev, CTRL_EXT);
> + break;
> + case bp_540:
> + ctrl = BP10G_READ_REG(pbpctl_dev, ESDP);
> + break;
> + case bp_10g9:
> + pbpctl_dev_c = get_status_port(pbpctl_dev);
> + if (!pbpctl_dev_c)
> + return;
> + ctrl = BP10G_READ_REG(pbpctl_dev_c, ESDP);
> + break;
> + default:
> + break;
> + }
> +
> + while (i--) {
> + ctrl_val = (value >> i) & 0x1;
> + if (ctrl_val) {
[snip]
if (ctrl_val)
helper_one(...);
else
helper_two(...);
It should help with:
[...]
> + case bp_fiber5:
> + BPCTL_BP_WRITE_REG(pbpctl_dev, CTRL, (ctrl_ext |
> + BPCTLI_CTRL_EXT_MCLK_DIR5
> + |
> + BPCTLI_CTRL_EXT_MDIO_DIR5
> + |
> + BPCTLI_CTRL_EXT_MDIO_DATA5
> + |
> + BPCTLI_CTRL_EXT_MCLK_DATA5));
as well as:
[...]
> + case bp_fiber5:
> + BPCTL_BP_WRITE_REG(pbpctl_dev, CTRL,
> + ((ctrl_ext |
> + BPCTLI_CTRL_EXT_MCLK_DIR5 |
> + BPCTLI_CTRL_EXT_MDIO_DIR5 |
> + BPCTLI_CTRL_EXT_MCLK_DATA5)
> + &
> + ~
> + (BPCTLI_CTRL_EXT_MDIO_DATA5)));
[...]
> + case bp_i80:
> + BPCTL_BP_WRITE_REG(pbpctl_dev, CTRL,
> + ((ctrl_ext |
> + BPCTLI_CTRL_EXT_MDIO_DIR80)
> + &
> + ~
> + (BPCTLI_CTRL_EXT_MDIO_DATA80)));
Excess parenthesis.
[...]
> +static s32 bp75_get_hw_semaphore_generic(struct bpctl_dev *pbpctl_dev)
> +{
[...]
> + if (i == timeout) {
> + /* Release semaphores */
> + bp75_put_hw_semaphore_generic(pbpctl_dev);
> + pr_err("Can't access the NVM\n");
> + ret_val = -1;
> + goto out;
> + }
> +
> + out:
Useless goto.
> + return ret_val;
> +}
[...]
> +static s32 bp75_acquire_phy(struct bpctl_dev *pbpctl_dev)
> +{
> + u16 mask = BPCTLI_SWFW_PHY0_SM;
> + u32 swfw_sync;
> + u32 swmask;
> + u32 fwmask;
> + s32 ret_val = 0;
> + s32 i = 0, timeout = 200;
> +
> + if ((pbpctl_dev->func == 1) || (pbpctl_dev->func == 3))
> + mask = BPCTLI_SWFW_PHY1_SM;
> +
> + swmask = mask;
> + fwmask = mask << 16;
> +
> + while (i < timeout) {
for (i = 0; i < timeout; i++)
I won't be able to read all this code and provide much valuable feedback
in a decent timeframe (huge macros, uint32_t and friends are still there).
The style may need more work but it could be necessary to split the big
pile of code into manageable patches (core features, config, more features,
more hardware support ?). Both your internal and external APIs may prove
real showstoppers (*lots* of EXPORT_SYMBOL, proc files and ioctl).
Imho you are pushing a lot of specialized use code and it will take
weeks rather than days of work before it settles down.
--
Ueimor
^ permalink raw reply
* Re: [PATCH next] r8169: add support for Byte Queue Limits
From: Tom Herbert @ 2014-09-29 21:49 UTC (permalink / raw)
To: Florian Westphal; +Cc: Linux Netdev List, Francois Romieu, Hayes Wang
In-Reply-To: <20140929211158.GA11633@breakpoint.cc>
On Mon, Sep 29, 2014 at 2:11 PM, Florian Westphal <fw@strlen.de> wrote:
> Tom Herbert <therbert@google.com> wrote:
>> Thanks for doing this! Can you add a little description in commit log
>> on test results, something that would indicate accounting is correct.
>
> Do you have any specific test in mind?
>
> I ran 'super_netperf 40' for 20 minutes without stalls/hangups and
> bql monitor seemed to look ok.
>
> Anything else I should use/test with?
>
Watch inflight and limit in the byte_queue_limits for the queue.
inflight must always go back to zero when link goes idle.
> Thanks,
> Florian
^ permalink raw reply
* Re: [PATCH v5 2/3] net: Add Keystone NetCP ethernet driver
From: Santosh Shilimkar @ 2014-09-29 21:47 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-arm-kernel, linux-kernel, robh+dt, grant.likely,
devicetree, sandeep_n, joe
In-Reply-To: <20140929.161253.1302757975839499212.davem@davemloft.net>
On Monday 29 September 2014 04:12 PM, David Miller wrote:
> From: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Date: Mon, 29 Sep 2014 16:02:24 -0400
>
>> We are badly missing mainline network driver support for the Keystone
>> and hence I request you to help here.
>
> It is absolutely not reasonable for you depend specifically upon me
> for top-level review of a given change, sorry.
>
Sorry I didn't mean you specifically to review everything. But a guidance
on how we can move forward. If you can suggest someone, we can try to
request them to have a look. The design of driver is just now standard
NIC card. We removed the controversial plug in framework after your
and Jamal's suggestion. Jamal already commented on the offload design of
the driver and we addressed them in the last version.
We can follow it up on any comments, suggestions quickly so do let us
know.
Regards,
Santosh
^ permalink raw reply
* Re: [PATCH] net: Add ndo_gso_check
From: Tom Herbert @ 2014-09-29 21:38 UTC (permalink / raw)
To: Or Gerlitz
Cc: Jeff Kirsher, Alexander Duyck, David Miller, Linux Netdev List,
Thomas Graf, Pravin Shelar, John Fastabend
In-Reply-To: <CAJ3xEMhf6b75TQDFqEPMSXCpL9ToWG1q0OMPtrEuHNSm=-PSwQ@mail.gmail.com>
On Mon, Sep 29, 2014 at 2:10 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> On Mon, Sep 29, 2014 at 11:53 PM, Tom Herbert <therbert@google.com> wrote:
>> On Mon, Sep 29, 2014 at 12:59 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>> On Mon, Sep 29, 2014 at 6:50 AM, Tom Herbert <therbert@google.com> wrote:
>>>> Add ndo_gso_check which a device can define to indicate whether is
>>>> is capable of doing GSO on a packet. This funciton would be called from
>>>> the stack to determine whether software GSO is needed to be done.
>>>
>>> please, no...
>>>
>>> We should strive to have a model/architecture under which the driver
>>> can clearly advertize up something (bits, collection of bits,
>>> whatever) which the stack can run through some dec-sion making code
>>> and decide if to GSO yes/no, do it ala SW or ala HW. As life, this
>>> model need not be perfect and can be biased towards being
>>> simple/robust/conservative and developed incrementally.
>>>
>> Please make a specific proposal then.
>
> We 1st need to bring the system back into a consistent state, see my
> band-aid proposal. BTW, this band-aid might turn to be the basis for
> the longer term solution too. I admit that saying "no" is ten (100)
> times harder vs. say "let's do it this way", but IMHO the fm10k call
> chain I pointed on is what you are suggesting more-or-less and is
> no-no
>
I'd much rather have drivers do this, than inflict the stack with more
complexity. As you describe "the driver can clearly advertise up
something (bits, collection of bits, whatever) which the stack can run
through some dec-sion making code and decide if to GSO yes/no"-- seems
very complex to me. My proposed alternative is to just ask the driver
and they can implement whatever policy they want, stack should doesn't
care about the specifics, just needs an answer. Neither does this
necessarily mean that driver needs to inspect packet, for instance I
suspect that just be looking at inner header lengths and skb->protocol
being TEB would be standard check to match VXLAN.
In any case, if you can formulate your proposal in a patch that would
be very helpful.
>
>>> We certainly don't want each driver that support some sort of HW
>>> offloads for tunnels (today we have 4-5, tomorrow maybe 40...) to
>>> implement their super sophisticated/efficient "dig in a packet and
>>> tell myself if I can GSO/CSUM it or not" code, while repeating (expect
>>> many copy/paste bugs...) the other drivers' code.
>>>
>> I would encourage vendors to implement protocol agnostic, generic
>> mechanisms so that they don't need to dig into the packet.
>
> M2.
>
> But wait, in Linux we support 20y old HW and it seems now will likely
> to continue doing so for maybe the next 10 or 20 coming years - I
> recall that one of the sessions in the coming Linux Con/KVM Forum/LPC
> conference series deals with practices to test kernel changes over
> driver code for which the HW is totally out of stock. Do we even have
> EOL procedure for upstream HW drivers...?
>
> So with this in mind, new NIC HW can (and should) definitely be
> developed along design guidelines such as being fully generic and
> simple. But, fact is that we have HW today and more coming up which is
> not 100% along this vision and their drivers are upstream and we want
> the kernel to be generic and hopefully using robust/simple SW model,
> right?
>
> Or.
^ permalink raw reply
* Re: [PATCH v1 5/5] driver-core: add driver asynchronous probe support
From: Tejun Heo @ 2014-09-29 21:26 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: gregkh, Luis R. Rodriguez, dmitry.torokhov, tiwai, arjan, teg,
rmilasan, werner, oleg, hare, bpoirier, santosh, pmladek, dbueso,
linux-kernel, Tetsuo Handa, Joseph Salisbury, Kay Sievers,
One Thousand Gnomes, Tim Gardner, Pierre Fersing, Andrew Morton,
Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
Abhijit Mahajan, Casey Leedom
In-Reply-To: <20140929212208.GV17349@wotan.suse.de>
Hello, Luis.
On Mon, Sep 29, 2014 at 11:22:08PM +0200, Luis R. Rodriguez wrote:
> > > + /* For now lets avoid stupid bug reports */
> > > + if (!strcmp(bus->name, "pci") ||
> > > + !strcmp(bus->name, "pci_express") ||
> > > + !strcmp(bus->name, "hid") ||
> > > + !strcmp(bus->name, "sdio") ||
> > > + !strcmp(bus->name, "gameport") ||
> > > + !strcmp(bus->name, "mmc") ||
> > > + !strcmp(bus->name, "i2c") ||
> > > + !strcmp(bus->name, "platform") ||
> > > + !strcmp(bus->name, "usb"))
> > > + return true;
> >
> > Ugh... things like this tend to become permanent. Do we really need
> > this? And how are we gonna find out what's broken why w/o bug
> > reports?
>
> Yeah... well we have two options, one is have something like this to
> at least make it generally useful or remove this and let folks who
> care start fixing async for all modules. The downside to removing
> this is it makes async probe pretty much useless on most systems
> right now, it would mean systemd would have to probably consider
> the list above if they wanted to start using this without expecting
> systems to not work.
So, I'd much prefer blacklist approach if something like this is a
necessity. That way, we'd at least know what doesn't work.
Thanks.
--
tejun
^ permalink raw reply
* Re: IPv6 FIB related crash with MACVLANs in 3.9.11+ kernel.
From: Ben Greear @ 2014-09-29 21:24 UTC (permalink / raw)
To: Cong Wang; +Cc: Hannes Frederic Sowa, Hongmei Li, netdev
In-Reply-To: <CAHA+R7O_xghD4a5RD=omhnuMD72pRmKU9VMoxPUVuGcu682jBg@mail.gmail.com>
On 09/29/2014 01:39 PM, Cong Wang wrote:
> On Mon, Sep 29, 2014 at 12:48 PM, Ben Greear <greearb@candelatech.com> wrote:
>>
>> We are going to be running up to 20 concurrent scripts that
>> will be dumping routes & ips, and configuring ip addresses
>> and routes during bringup.
>>
>> I don't have a simple stand-alone way to reproduce this,
>> but at least when we reported the problem is was very easy
>> for us to reproduce with our tool.
>>
>> Maybe 20 scripts running in parallel that randomly configured and dumped routes
>> and ip addresses on random interfaces would do the trick?
>>
>
> I think the most important question is why is this related with macvlan?
> IPv6 is L3 while macvlan pure L2, if this is a IPv6 routing bug, it should
> not be limited to macvlan.
We saw it using mac-vlans on top of ixgbe. Probably it can be reproduced
elsewhere, but since we had a good test case, we didn't bother trying lots
of other combinations.
Just enabling some debug code caused the problem to be much harder
to reproduce, so it is probably some sort of race. Maybe lots of mac-vlans
make it easier to hit.
> What does your IPv6 routing table look like? And how do you configure those
> macvlan interfaces?
Each interface would have it's own routing table, with at least as subnet
route. I'm not sure we were adding a default gateway or not.
The main routing table would also have some auto-created routes
I think.
It is configured using 'ip' to add and dump routes.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH v1 5/5] driver-core: add driver asynchronous probe support
From: Luis R. Rodriguez @ 2014-09-29 21:22 UTC (permalink / raw)
To: Tejun Heo, gregkh
Cc: Luis R. Rodriguez, dmitry.torokhov, tiwai, arjan, teg, rmilasan,
werner, oleg, hare, bpoirier, santosh, pmladek, dbueso,
linux-kernel, Tetsuo Handa, Joseph Salisbury, Kay Sievers,
One Thousand Gnomes, Tim Gardner, Pierre Fersing, Andrew Morton,
Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
Abhijit Mahajan, Casey Leedom, Hariprasad S <har
In-Reply-To: <20140928150329.GC5023@mtj.dyndns.org>
On Sun, Sep 28, 2014 at 11:03:29AM -0400, Tejun Heo wrote:
> Hello,
>
> On Fri, Sep 26, 2014 at 02:57:17PM -0700, Luis R. Rodriguez wrote:
> ...
> > Systemd should consider enabling async probe on device drivers
> > it loads through systemd-udev but probably does not want to
> > enable it for modules loaded through systemd-modules-load
> > (modules-load.d). At least on my booting enablign async probe
> > for all modules fails to boot as such in order to make this
>
> Did you find out why boot failed with those modules?
No, it seems this was early in boot and I haven't been able to capture the logs
yet of the faults. More on this below.
> > a bit more useful we whitelist a few buses where it should be
> > at least in theory safe to try to enable async probe. This
> > way even if systemd tried to ask to enable async probe for all
> > its device drivers the kernel won't blindly do this. We also
> > have the sync_probe flag which device drivers can themselves
> > enable *iff* its known the device driver should never async
> > probe.
> >
> > In order to help *test* things folks can use the bus.safe_mod_async_probe=1
> > kernel parameter which will work as if userspace would have
> > requested all modules to load with async probe. Daring folks can
> > also use bus.force_mod_async_probe=1 which will enable asynch probe
> > even on buses not tested in any way yet, if you use that though
> > you're on your own.
>
> If those two knobs are meant for debugging, let's please make that
> fact immediately evident. e.g. Make them ugly boot params like
> "__DEVEL__driver_force_mod_async_probe". Devel/debug options ending
> up becoming stable interface are really nasty.
Sure make sense, I wasn't quite sure how to make this quite clear,
a naming convention seems good to me but I also had added at least
a print about this on the log. Ideally I think a TAIN_DEBUG would
be best and it seems it could be useful for many other cases in
the kernel, we could also just re-use TAINT_CRAP as well. Thoughts?
Greg?
> > +struct driver_attach_work {
> > + struct work_struct work;
> > + struct device_driver *driver;
> > +};
> > +
> > struct driver_private {
> > struct kobject kobj;
> > struct klist klist_devices;
> > struct klist_node knode_bus;
> > struct module_kobject *mkobj;
> > + struct driver_attach_work *attach_work;
> > struct device_driver *driver;
> > };
>
> How many bytes are we saving by allocating it separately?
This saves us 24 bytes per device driver.
> Can't we just embed it in driver_private?
We sure can and it is my preference to do that as well but just
in case I wanted to take the alternative space saving approach
as well and let folks decide. There's also the technical aspect
of hiding that data structure from drivers, and that may be worth
to do but I personally also prefer the simplicity of stuffing
it on the public data structure, as you noted below we could then also
unconditionally flush_work() on __device_release_driver().
Greg, any preference?
> > +static void driver_attach_workfn(struct work_struct *work)
> > +{
> > + int ret;
> > + struct driver_attach_work *attach_work =
> > + container_of(work, struct driver_attach_work, work);
> > + struct device_driver *drv = attach_work->driver;
> > + ktime_t calltime, delta, rettime;
> > + unsigned long long duration;
>
> This could just be a personal preference but I think it's easier to
> read if local vars w/ initializers come before the ones w/o.
We gotta standardize on *something*, I tend to declare them
in the order in which they are used, in this case I failed to
list calltime first, but yeah I'll put initialized first, I
don't care much.
> > +
> > + calltime = ktime_get();
> > +
> > + ret = driver_attach(drv);
> > + if (ret != 0) {
> > + remove_driver_private(drv);
> > + bus_put(drv->bus);
> > + }
> > +
> > + rettime = ktime_get();
> > + delta = ktime_sub(rettime, calltime);
> > + duration = (unsigned long long) ktime_to_ns(delta) >> 10;
> > +
> > + pr_debug("bus: '%s': add driver %s attach completed after %lld usecs\n",
> > + drv->bus->name, drv->name, duration);
>
> Why do we have the above printout for async path but not sync path?
> It's kinda weird for the code path to diverge like this. Shouldn't
> the only difference be the context probes are running from?
Yeah sure, I'll remove this, it was useful for me for testing purposes
in evaluation against kthreads / sync runs, but that certainly was mostly
for debugging.
> ...
> > +static bool drv_enable_async_probe(struct device_driver *drv,
> > + struct bus_type *bus)
> > +{
> > + struct module *mod;
> > +
> > + if (!drv->owner || drv->sync_probe)
> > + return false;
> > +
> > + if (force_mod_async)
> > + return true;
> > +
> > + mod = drv->owner;
> > + if (!safe_mod_async && !mod->async_probe_requested)
> > + return false;
> > +
> > + /* For now lets avoid stupid bug reports */
> > + if (!strcmp(bus->name, "pci") ||
> > + !strcmp(bus->name, "pci_express") ||
> > + !strcmp(bus->name, "hid") ||
> > + !strcmp(bus->name, "sdio") ||
> > + !strcmp(bus->name, "gameport") ||
> > + !strcmp(bus->name, "mmc") ||
> > + !strcmp(bus->name, "i2c") ||
> > + !strcmp(bus->name, "platform") ||
> > + !strcmp(bus->name, "usb"))
> > + return true;
>
> Ugh... things like this tend to become permanent. Do we really need
> this? And how are we gonna find out what's broken why w/o bug
> reports?
Yeah... well we have two options, one is have something like this to
at least make it generally useful or remove this and let folks who
care start fixing async for all modules. The downside to removing
this is it makes async probe pretty much useless on most systems
right now, it would mean systemd would have to probably consider
the list above if they wanted to start using this without expecting
systems to not work.
Let me know what is preferred.
> > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > index e4ffbcf..7999aba 100644
> > --- a/drivers/base/dd.c
> > +++ b/drivers/base/dd.c
> > @@ -507,6 +507,13 @@ static void __device_release_driver(struct device *dev)
> >
> > drv = dev->driver;
> > if (drv) {
> > + if (drv->owner && !drv->sync_probe) {
> > + struct module *mod = drv->owner;
> > + struct driver_private *priv = drv->p;
> > +
> > + if (mod->async_probe_requested)
> > + flush_work(&priv->attach_work->work);
>
> This can be unconditional flush_work(&priv->attach_work) if attach_work
> isn't separately allocated.
Indeed.
> > static int unknown_module_param_cb(char *param, char *val, const char *modname,
> > void *arg)
> > {
> > + int ret;
> > + struct module *mod = arg;
>
> Ditto with the order of definitions.
Amended.
> Generally looks good to me.
>
> Thanks a lot for doing this! :)
Thanks for the review and pointers so far.
Luis
^ permalink raw reply
* Re: [PATCH next] r8169: add support for Byte Queue Limits
From: Florian Westphal @ 2014-09-29 21:11 UTC (permalink / raw)
To: Tom Herbert
Cc: Florian Westphal, Linux Netdev List, Francois Romieu, Hayes Wang
In-Reply-To: <CA+mtBx95KFr=3h_TZWBeM6e7EF4sKAmjRpz5vLxGwi8xC+qQuw@mail.gmail.com>
Tom Herbert <therbert@google.com> wrote:
> Thanks for doing this! Can you add a little description in commit log
> on test results, something that would indicate accounting is correct.
Do you have any specific test in mind?
I ran 'super_netperf 40' for 20 minutes without stalls/hangups and
bql monitor seemed to look ok.
Anything else I should use/test with?
Thanks,
Florian
^ permalink raw reply
* Re: [PATCH] net: Add ndo_gso_check
From: Or Gerlitz @ 2014-09-29 21:10 UTC (permalink / raw)
To: Tom Herbert
Cc: Jeff Kirsher, Alexander Duyck, David Miller, Linux Netdev List,
Thomas Graf, Pravin Shelar, John Fastabend
In-Reply-To: <CA+mtBx8r05=qs2J58LFq56762tFB7wF4Dsr3C5TC9w5qQP5jGw@mail.gmail.com>
On Mon, Sep 29, 2014 at 11:53 PM, Tom Herbert <therbert@google.com> wrote:
> On Mon, Sep 29, 2014 at 12:59 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>> On Mon, Sep 29, 2014 at 6:50 AM, Tom Herbert <therbert@google.com> wrote:
>>> Add ndo_gso_check which a device can define to indicate whether is
>>> is capable of doing GSO on a packet. This funciton would be called from
>>> the stack to determine whether software GSO is needed to be done.
>>
>> please, no...
>>
>> We should strive to have a model/architecture under which the driver
>> can clearly advertize up something (bits, collection of bits,
>> whatever) which the stack can run through some dec-sion making code
>> and decide if to GSO yes/no, do it ala SW or ala HW. As life, this
>> model need not be perfect and can be biased towards being
>> simple/robust/conservative and developed incrementally.
>>
> Please make a specific proposal then.
We 1st need to bring the system back into a consistent state, see my
band-aid proposal. BTW, this band-aid might turn to be the basis for
the longer term solution too. I admit that saying "no" is ten (100)
times harder vs. say "let's do it this way", but IMHO the fm10k call
chain I pointed on is what you are suggesting more-or-less and is
no-no
>> We certainly don't want each driver that support some sort of HW
>> offloads for tunnels (today we have 4-5, tomorrow maybe 40...) to
>> implement their super sophisticated/efficient "dig in a packet and
>> tell myself if I can GSO/CSUM it or not" code, while repeating (expect
>> many copy/paste bugs...) the other drivers' code.
>>
> I would encourage vendors to implement protocol agnostic, generic
> mechanisms so that they don't need to dig into the packet.
M2.
But wait, in Linux we support 20y old HW and it seems now will likely
to continue doing so for maybe the next 10 or 20 coming years - I
recall that one of the sessions in the coming Linux Con/KVM Forum/LPC
conference series deals with practices to test kernel changes over
driver code for which the HW is totally out of stock. Do we even have
EOL procedure for upstream HW drivers...?
So with this in mind, new NIC HW can (and should) definitely be
developed along design guidelines such as being fully generic and
simple. But, fact is that we have HW today and more coming up which is
not 100% along this vision and their drivers are upstream and we want
the kernel to be generic and hopefully using robust/simple SW model,
right?
Or.
^ permalink raw reply
* [PATCH V2 next] r8169: add support for Byte Queue Limits
From: Florian Westphal @ 2014-09-29 21:07 UTC (permalink / raw)
To: netdev; +Cc: brouer, Florian Westphal, Francois Romieu, Hayes Wang
tested on RTL8168d/8111d.
Cc: Francois Romieu <romieu@fr.zoreil.com>
Cc: Hayes Wang <hayeswang@realtek.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Eric Dumazet <edumazet@google.com>
---
changes since v1:
unsigned int foo, not unsigned foo (thanks Eric!)
drivers/net/ethernet/realtek/r8169.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 1d81238..51b6dc2 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -4712,6 +4712,8 @@ static void rtl_hw_reset(struct rtl8169_private *tp)
RTL_W8(ChipCmd, CmdReset);
rtl_udelay_loop_wait_low(tp, &rtl_chipcmd_cond, 100, 100);
+
+ netdev_reset_queue(tp->dev);
}
static void rtl_request_uncached_firmware(struct rtl8169_private *tp)
@@ -6592,6 +6594,8 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
txd->opts2 = cpu_to_le32(opts[1]);
+ netdev_sent_queue(dev, skb->len);
+
skb_tx_timestamp(skb);
wmb();
@@ -6691,6 +6695,7 @@ static void rtl8169_pcierr_interrupt(struct net_device *dev)
static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
{
unsigned int dirty_tx, tx_left;
+ unsigned int bytes_compl = 0, pkts_compl = 0;
dirty_tx = tp->dirty_tx;
smp_rmb();
@@ -6709,10 +6714,8 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
rtl8169_unmap_tx_skb(&tp->pci_dev->dev, tx_skb,
tp->TxDescArray + entry);
if (status & LastFrag) {
- u64_stats_update_begin(&tp->tx_stats.syncp);
- tp->tx_stats.packets++;
- tp->tx_stats.bytes += tx_skb->skb->len;
- u64_stats_update_end(&tp->tx_stats.syncp);
+ pkts_compl++;
+ bytes_compl += tx_skb->skb->len;
dev_kfree_skb_any(tx_skb->skb);
tx_skb->skb = NULL;
}
@@ -6721,6 +6724,13 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
}
if (tp->dirty_tx != dirty_tx) {
+ netdev_completed_queue(tp->dev, pkts_compl, bytes_compl);
+
+ u64_stats_update_begin(&tp->tx_stats.syncp);
+ tp->tx_stats.packets += pkts_compl;
+ tp->tx_stats.bytes += bytes_compl;
+ u64_stats_update_end(&tp->tx_stats.syncp);
+
tp->dirty_tx = dirty_tx;
/* Sync with rtl8169_start_xmit:
* - publish dirty_tx ring index (write barrier)
--
1.8.1.5
^ permalink raw reply related
* Re: [PATCH next] r8169: add support for Byte Queue Limits
From: Tom Herbert @ 2014-09-29 21:05 UTC (permalink / raw)
To: Florian Westphal; +Cc: Linux Netdev List, Francois Romieu, Hayes Wang
In-Reply-To: <1412024209-25468-1-git-send-email-fw@strlen.de>
Hi Florian,
Thanks for doing this! Can you add a little description in commit log
on test results, something that would indicate accounting is correct.
Thanks,
Tom
On Mon, Sep 29, 2014 at 1:56 PM, Florian Westphal <fw@strlen.de> wrote:
> tested on RTL8168d/8111d.
>
> Cc: Francois Romieu <romieu@fr.zoreil.com>
> Cc: Hayes Wang <hayeswang@realtek.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> drivers/net/ethernet/realtek/r8169.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index 1d81238..73c6ad1 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -4712,6 +4712,8 @@ static void rtl_hw_reset(struct rtl8169_private *tp)
> RTL_W8(ChipCmd, CmdReset);
>
> rtl_udelay_loop_wait_low(tp, &rtl_chipcmd_cond, 100, 100);
> +
> + netdev_reset_queue(tp->dev);
> }
>
> static void rtl_request_uncached_firmware(struct rtl8169_private *tp)
> @@ -6592,6 +6594,8 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
>
> txd->opts2 = cpu_to_le32(opts[1]);
>
> + netdev_sent_queue(dev, skb->len);
> +
> skb_tx_timestamp(skb);
>
> wmb();
> @@ -6691,6 +6695,7 @@ static void rtl8169_pcierr_interrupt(struct net_device *dev)
> static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
> {
> unsigned int dirty_tx, tx_left;
> + unsigned bytes_compl = 0, pkts_compl = 0;
>
> dirty_tx = tp->dirty_tx;
> smp_rmb();
> @@ -6709,10 +6714,8 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
> rtl8169_unmap_tx_skb(&tp->pci_dev->dev, tx_skb,
> tp->TxDescArray + entry);
> if (status & LastFrag) {
> - u64_stats_update_begin(&tp->tx_stats.syncp);
> - tp->tx_stats.packets++;
> - tp->tx_stats.bytes += tx_skb->skb->len;
> - u64_stats_update_end(&tp->tx_stats.syncp);
> + pkts_compl++;
> + bytes_compl += tx_skb->skb->len;
> dev_kfree_skb_any(tx_skb->skb);
> tx_skb->skb = NULL;
> }
> @@ -6721,6 +6724,13 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
> }
>
> if (tp->dirty_tx != dirty_tx) {
> + netdev_completed_queue(tp->dev, pkts_compl, bytes_compl);
> +
> + u64_stats_update_begin(&tp->tx_stats.syncp);
> + tp->tx_stats.packets += pkts_compl;
> + tp->tx_stats.bytes += bytes_compl;
> + u64_stats_update_end(&tp->tx_stats.syncp);
> +
> tp->dirty_tx = dirty_tx;
> /* Sync with rtl8169_start_xmit:
> * - publish dirty_tx ring index (write barrier)
> --
> 1.8.1.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH next] r8169: add support for Byte Queue Limits
From: Eric Dumazet @ 2014-09-29 21:03 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev, Francois Romieu, Hayes Wang
In-Reply-To: <1412024209-25468-1-git-send-email-fw@strlen.de>
On Mon, 2014-09-29 at 22:56 +0200, Florian Westphal wrote:
> tested on RTL8168d/8111d.
One small nit :
> @@ -6691,6 +6695,7 @@ static void rtl8169_pcierr_interrupt(struct net_device *dev)
> static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
> {
> unsigned int dirty_tx, tx_left;
> + unsigned bytes_compl = 0, pkts_compl = 0;
unsigned int bytes_compl = 0, pkts_compl = 0;
^ permalink raw reply
* Re: [PATCHv8 net-next 2/4] sunvnet: make transmit path zero-copy in the kernel
From: David L Stevens @ 2014-09-29 21:02 UTC (permalink / raw)
To: David Miller; +Cc: netdev, sowmini.varadhan, raghuram.kothakota
In-Reply-To: <20140929.165440.1575595209745066192.davem@davemloft.net>
On 09/29/2014 04:54 PM, David Miller wrote:
> From: David L Stevens <david.stevens@oracle.com>
>> For example, I could leave the "normal" path like this and just start/mod
>> a timer to do it after 1 sec if we haven't done it through start_xmit. Do
>> you think that's sufficiently short?
>
> To be honest I'm not %100 sure.
Since it won't actually fire when the network is loaded, maybe a
shorter timer won't hurt. I'll try it out.
+-DLS
^ permalink raw reply
* Re: [PATCH next] r8169: add support for Byte Queue Limits
From: Eric Dumazet @ 2014-09-29 21:00 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev, Francois Romieu, Hayes Wang
In-Reply-To: <1412024209-25468-1-git-send-email-fw@strlen.de>
On Mon, 2014-09-29 at 22:56 +0200, Florian Westphal wrote:
> tested on RTL8168d/8111d.
>
> Cc: Francois Romieu <romieu@fr.zoreil.com>
> Cc: Hayes Wang <hayeswang@realtek.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> drivers/net/ethernet/realtek/r8169.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
Thanks Florian, this looks good to me.
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* [PATCH next] r8169: add support for Byte Queue Limits
From: Florian Westphal @ 2014-09-29 20:56 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal, Francois Romieu, Hayes Wang
tested on RTL8168d/8111d.
Cc: Francois Romieu <romieu@fr.zoreil.com>
Cc: Hayes Wang <hayeswang@realtek.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
drivers/net/ethernet/realtek/r8169.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 1d81238..73c6ad1 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -4712,6 +4712,8 @@ static void rtl_hw_reset(struct rtl8169_private *tp)
RTL_W8(ChipCmd, CmdReset);
rtl_udelay_loop_wait_low(tp, &rtl_chipcmd_cond, 100, 100);
+
+ netdev_reset_queue(tp->dev);
}
static void rtl_request_uncached_firmware(struct rtl8169_private *tp)
@@ -6592,6 +6594,8 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
txd->opts2 = cpu_to_le32(opts[1]);
+ netdev_sent_queue(dev, skb->len);
+
skb_tx_timestamp(skb);
wmb();
@@ -6691,6 +6695,7 @@ static void rtl8169_pcierr_interrupt(struct net_device *dev)
static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
{
unsigned int dirty_tx, tx_left;
+ unsigned bytes_compl = 0, pkts_compl = 0;
dirty_tx = tp->dirty_tx;
smp_rmb();
@@ -6709,10 +6714,8 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
rtl8169_unmap_tx_skb(&tp->pci_dev->dev, tx_skb,
tp->TxDescArray + entry);
if (status & LastFrag) {
- u64_stats_update_begin(&tp->tx_stats.syncp);
- tp->tx_stats.packets++;
- tp->tx_stats.bytes += tx_skb->skb->len;
- u64_stats_update_end(&tp->tx_stats.syncp);
+ pkts_compl++;
+ bytes_compl += tx_skb->skb->len;
dev_kfree_skb_any(tx_skb->skb);
tx_skb->skb = NULL;
}
@@ -6721,6 +6724,13 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
}
if (tp->dirty_tx != dirty_tx) {
+ netdev_completed_queue(tp->dev, pkts_compl, bytes_compl);
+
+ u64_stats_update_begin(&tp->tx_stats.syncp);
+ tp->tx_stats.packets += pkts_compl;
+ tp->tx_stats.bytes += bytes_compl;
+ u64_stats_update_end(&tp->tx_stats.syncp);
+
tp->dirty_tx = dirty_tx;
/* Sync with rtl8169_start_xmit:
* - publish dirty_tx ring index (write barrier)
--
1.8.1.5
^ permalink raw reply related
* Re: [PATCHv8 net-next 2/4] sunvnet: make transmit path zero-copy in the kernel
From: David Miller @ 2014-09-29 20:54 UTC (permalink / raw)
To: david.stevens; +Cc: netdev, sowmini.varadhan, raghuram.kothakota
In-Reply-To: <5429C498.1000705@oracle.com>
From: David L Stevens <david.stevens@oracle.com>
Date: Mon, 29 Sep 2014 16:44:08 -0400
> On 09/29/2014 04:29 PM, David Miller wrote:
>
>>
>> It doesn't work to liberate SKBs in the TX ring purely from the
>> ->ndo_start_xmit() method.
>>
>> All SKBs given to a device must be liberated in a finite, short,
>> amount of time.
>
> I did consider putting a garbage-collector via timer on them, since
> we got such a boost from not ACKing every packet. I guess the question
> is "how short?"
>
> For example, I could leave the "normal" path like this and just start/mod
> a timer to do it after 1 sec if we haven't done it through start_xmit. Do
> you think that's sufficiently short?
To be honest I'm not %100 sure.
SKB liberation frees up space in the TCP socket send buffer, so... but
arguably if the connection actually cares and is in steady state then
we'll be liberating via the ->ndo_start_xmit() flush you do here.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox