* RT2070 usb wifi is not working with compat-wireless-3.1.1-1
From: Dipinkumar @ 2012-05-16 13:11 UTC (permalink / raw)
To: netdev
Hi,
I am using RT2070 usb wifi with compat-wireless-3.1.1-1 on my imx23
embedded board.My board have linux-2.3.35.3 kernel.When kernel is
loading,giving the following error
phy0 -> rt2x00usb_vendor_request: Error - Vendor Request 0x07 failed
for offset 0x0580 with error -19.
phy0 -> rt2800_init_eeprom: Error - Invalid RT chipset detected.
phy0 -> rt2x00lib_probe_dev: Error - Failed to allocate device.
usbcore: registered new interface driver rt2800usb
hub 1-0:1.0: unable to enumerate USB device on port 1
can you help me to solve this issue?
Regards,
Dipin kumar
^ permalink raw reply
* [PATCH] drop_monitor: convert to modular building
From: Neil Horman @ 2012-05-16 14:27 UTC (permalink / raw)
To: netdev; +Cc: Neil Horman, David S. Miller
When I first wrote drop monitor I wrote it to just build monolithically. There
is no reason it can't be built modularly as well, so lets give it that
flexibiity.
I've tested this by building it as both a module and monolithically, and it
seems to work quite well
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: "David S. Miller" <davem@davemloft.net>
---
net/Kconfig | 2 +-
net/core/drop_monitor.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/net/Kconfig b/net/Kconfig
index e07272d..76ad6fa 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -295,7 +295,7 @@ config NET_TCPPROBE
module will be called tcp_probe.
config NET_DROP_MONITOR
- boolean "Network packet drop alerting service"
+ tristate "Network packet drop alerting service"
depends on INET && EXPERIMENTAL && TRACEPOINTS
---help---
This feature provides an alerting service to userspace in the
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 5c3c81a..145f399 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -22,6 +22,7 @@
#include <linux/timer.h>
#include <linux/bitops.h>
#include <linux/slab.h>
+#include <linux/module.h>
#include <net/genetlink.h>
#include <net/netevent.h>
@@ -223,9 +224,15 @@ static int set_all_monitor_traces(int state)
switch (state) {
case TRACE_ON:
+ if (!try_module_get(THIS_MODULE)) {
+ rc = -EINVAL;
+ break;
+ }
+
rc |= register_trace_kfree_skb(trace_kfree_skb_hit, NULL);
rc |= register_trace_napi_poll(trace_napi_poll_hit, NULL);
break;
+
case TRACE_OFF:
rc |= unregister_trace_kfree_skb(trace_kfree_skb_hit, NULL);
rc |= unregister_trace_napi_poll(trace_napi_poll_hit, NULL);
@@ -241,6 +248,9 @@ static int set_all_monitor_traces(int state)
kfree_rcu(new_stat, rcu);
}
}
+
+ module_put(THIS_MODULE);
+
break;
default:
rc = 1;
@@ -383,4 +393,38 @@ out:
return rc;
}
-late_initcall(init_net_drop_monitor);
+static void exit_net_drop_monitor(void)
+{
+ struct per_cpu_dm_data *data;
+ int cpu;
+
+ if (unregister_netdevice_notifier(&dropmon_net_notifier))
+ printk(KERN_WARNING "Unable to unregiser dropmon notifer\n");
+
+ /*
+ * Because of the module_get/put we do in the trace state change path
+ * we are guarnateed not to have any current users when we get here
+ * all we need to do is make sure that we don't have any running timers
+ * or pending schedule calls
+ */
+
+ for_each_present_cpu(cpu) {
+ data = &per_cpu(dm_cpu_data, cpu);
+ del_timer(&data->send_timer);
+ cancel_work_sync(&data->dm_alert_work);
+ /*
+ * At this point, we should have exclusive access
+ * to this struct and can free the skb inside it
+ */
+ kfree_skb(data->skb);
+ }
+
+ if (genl_unregister_family(&net_drop_monitor_family))
+ printk(KERN_WARNING "Unable to unregister drop monitor socket family\n");
+}
+
+module_init(init_net_drop_monitor);
+module_exit(exit_net_drop_monitor);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Neil Horman <nhorman@tuxdriver.com>");
--
1.7.7.6
^ permalink raw reply related
* [PATCH net-next] fq_codel: should use qdisc backlog as threshold
From: Eric Dumazet @ 2012-05-16 14:39 UTC (permalink / raw)
To: David Miller
Cc: netdev, Kathleen Nichols, Dave Taht, Van Jacobson, codel, bloat
From: Eric Dumazet <edumazet@google.com>
codel_should_drop() logic allows a packet being not dropped if queue
size is under max packet size.
In fq_codel, we have two possible backlogs : The qdisc global one, and
the flow local one.
The meaningful one for codel_should_drop() should be the global backlog,
not the per flow one, so that thin flows can have a non zero drop/mark
probability.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Dave Taht <dave.taht@bufferbloat.net>
Cc: Kathleen Nichols <nichols@pollere.com>
Cc: Van Jacobson <van@pollere.net>
---
include/net/codel.h | 15 +++++++--------
net/sched/sch_codel.c | 4 ++--
net/sched/sch_fq_codel.c | 5 +++--
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/include/net/codel.h b/include/net/codel.h
index 7546517..550debf 100644
--- a/include/net/codel.h
+++ b/include/net/codel.h
@@ -205,7 +205,7 @@ static codel_time_t codel_control_law(codel_time_t t,
static bool codel_should_drop(const struct sk_buff *skb,
- unsigned int *backlog,
+ struct Qdisc *sch,
struct codel_vars *vars,
struct codel_params *params,
struct codel_stats *stats,
@@ -219,13 +219,13 @@ static bool codel_should_drop(const struct sk_buff *skb,
}
vars->ldelay = now - codel_get_enqueue_time(skb);
- *backlog -= qdisc_pkt_len(skb);
+ sch->qstats.backlog -= qdisc_pkt_len(skb);
if (unlikely(qdisc_pkt_len(skb) > stats->maxpacket))
stats->maxpacket = qdisc_pkt_len(skb);
if (codel_time_before(vars->ldelay, params->target) ||
- *backlog <= stats->maxpacket) {
+ sch->qstats.backlog <= stats->maxpacket) {
/* went below - stay below for at least interval */
vars->first_above_time = 0;
return false;
@@ -249,8 +249,7 @@ static struct sk_buff *codel_dequeue(struct Qdisc *sch,
struct codel_params *params,
struct codel_vars *vars,
struct codel_stats *stats,
- codel_skb_dequeue_t dequeue_func,
- u32 *backlog)
+ codel_skb_dequeue_t dequeue_func)
{
struct sk_buff *skb = dequeue_func(vars, sch);
codel_time_t now;
@@ -261,7 +260,7 @@ static struct sk_buff *codel_dequeue(struct Qdisc *sch,
return skb;
}
now = codel_get_time();
- drop = codel_should_drop(skb, backlog, vars, params, stats, now);
+ drop = codel_should_drop(skb, sch, vars, params, stats, now);
if (vars->dropping) {
if (!drop) {
/* sojourn time below target - leave dropping state */
@@ -292,7 +291,7 @@ static struct sk_buff *codel_dequeue(struct Qdisc *sch,
qdisc_drop(skb, sch);
stats->drop_count++;
skb = dequeue_func(vars, sch);
- if (!codel_should_drop(skb, backlog,
+ if (!codel_should_drop(skb, sch,
vars, params, stats, now)) {
/* leave dropping state */
vars->dropping = false;
@@ -313,7 +312,7 @@ static struct sk_buff *codel_dequeue(struct Qdisc *sch,
stats->drop_count++;
skb = dequeue_func(vars, sch);
- drop = codel_should_drop(skb, backlog, vars, params,
+ drop = codel_should_drop(skb, sch, vars, params,
stats, now);
}
vars->dropping = true;
diff --git a/net/sched/sch_codel.c b/net/sched/sch_codel.c
index 213ef60..2f9ab17 100644
--- a/net/sched/sch_codel.c
+++ b/net/sched/sch_codel.c
@@ -77,8 +77,8 @@ static struct sk_buff *codel_qdisc_dequeue(struct Qdisc *sch)
struct codel_sched_data *q = qdisc_priv(sch);
struct sk_buff *skb;
- skb = codel_dequeue(sch, &q->params, &q->vars, &q->stats,
- dequeue, &sch->qstats.backlog);
+ skb = codel_dequeue(sch, &q->params, &q->vars, &q->stats, dequeue);
+
/* We cant call qdisc_tree_decrease_qlen() if our qlen is 0,
* or HTB crashes. Defer it for next round.
*/
diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
index 337ff20..9fc1c62 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -217,13 +217,14 @@ static int fq_codel_enqueue(struct sk_buff *skb, struct Qdisc *sch)
*/
static struct sk_buff *dequeue(struct codel_vars *vars, struct Qdisc *sch)
{
+ struct fq_codel_sched_data *q = qdisc_priv(sch);
struct fq_codel_flow *flow;
struct sk_buff *skb = NULL;
flow = container_of(vars, struct fq_codel_flow, cvars);
if (flow->head) {
skb = dequeue_head(flow);
- sch->qstats.backlog -= qdisc_pkt_len(skb);
+ q->backlogs[flow - q->flows] -= qdisc_pkt_len(skb);
sch->q.qlen--;
}
return skb;
@@ -256,7 +257,7 @@ begin:
prev_ecn_mark = q->cstats.ecn_mark;
skb = codel_dequeue(sch, &q->cparams, &flow->cvars, &q->cstats,
- dequeue, &q->backlogs[flow - q->flows]);
+ dequeue);
flow->dropped += q->cstats.drop_count - prev_drop_count;
flow->dropped += q->cstats.ecn_mark - prev_ecn_mark;
^ permalink raw reply related
* e1000: dma_map_area overflow
From: Bernd Schubert @ 2012-05-16 14:44 UTC (permalink / raw)
To: netdev
I'm getting kernel panics from the e1000 driver here. Is this a known issue?
> fslab1 login: [89848.228249] Kernel panic - not syncing: dma_map_area overflow 66 bytes
> [89848.228251]
> [89848.236378] Pid: 14293, comm: fhgfs-storage/S Not tainted 3.3.6-debug #19
> [89848.243217] Call Trace:
> [89848.245703] <IRQ> [<ffffffff813fc3ed>] panic+0xbd/0x1df
> [89848.251169] [<ffffffff810433e6>] ? alloc_iommu+0xd6/0x160
> [89848.256703] [<ffffffff8104360b>] dma_map_area+0x12b/0x130
> [89848.262237] [<ffffffff81043ce3>] gart_map_page+0x93/0xb0
> [89848.267682] [<ffffffff810956ef>] ? local_clock+0x6f/0x80
> [89848.273137] [<ffffffffa00e564d>] e1000_tx_map+0x1bd/0x640 [e1000]
> [89848.279377] [<ffffffff81043c50>] ? dma_map_sg_nonforce+0x190/0x190
> [89848.285694] [<ffffffff813750a8>] ? sch_direct_xmit+0x38/0x2a0
> [89848.291584] [<ffffffffa00e5c75>] e1000_xmit_frame+0x1a5/0x5d0 [e1000]
> [89848.298159] [<ffffffff81357a60>] dev_hard_start_xmit+0x2b0/0x650
> [89848.304320] [<ffffffff8137516e>] sch_direct_xmit+0xfe/0x2a0
> [89848.310038] [<ffffffff81358205>] dev_queue_xmit+0x165/0x6d0
> [89848.315757] [<ffffffff813580cf>] ? dev_queue_xmit+0x2f/0x6d0
> [89848.321557] [<ffffffff813896c6>] ip_finish_output2+0x166/0x430
> [89848.327527] [<ffffffff813895f7>] ? ip_finish_output2+0x97/0x430
> [89848.333586] [<ffffffff8138af00>] ip_finish_output+0x60/0xf0
> [89848.339438] [<ffffffff8138afef>] ip_output+0x5f/0xd0
> [89848.344536] [<ffffffff813899bd>] ip_local_out+0x2d/0x90
> [89848.349899] [<ffffffff8138a1df>] ip_queue_xmit+0x1bf/0x580
> [89848.355524] [<ffffffff8138a05d>] ? ip_queue_xmit+0x3d/0x580
> [89848.361231] [<ffffffff810a73c7>] ? getnstimeofday+0x57/0xe0
> [89848.366940] [<ffffffff813a2d77>] tcp_transmit_skb+0x3f7/0x770
> [89848.372816] [<ffffffff813a52c0>] ? tcp_init_xmit_timers+0x30/0x30
> [89848.379047] [<ffffffff813a32ad>] tcp_send_ack+0xdd/0x130
> [89848.384495] [<ffffffff813a544e>] tcp_delack_timer+0x18e/0x250
> [89848.390374] [<ffffffff8106739f>] call_timer_fn+0x9f/0x250
> [89848.395909] [<ffffffff81067300>] ? del_timer+0x140/0x140
Thanks,
Bernd
^ permalink raw reply
* Re: [PATCH] drop_monitor: convert to modular building
From: Eric Dumazet @ 2012-05-16 14:48 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, David S. Miller
In-Reply-To: <1337178426-2470-1-git-send-email-nhorman@tuxdriver.com>
On Wed, 2012-05-16 at 10:27 -0400, Neil Horman wrote:
> When I first wrote drop monitor I wrote it to just build monolithically. There
> is no reason it can't be built modularly as well, so lets give it that
> flexibiity.
> + for_each_present_cpu(cpu) {
> + data = &per_cpu(dm_cpu_data, cpu);
> + del_timer(&data->send_timer);
> + cancel_work_sync(&data->dm_alert_work);
> + /*
> + * At this point, we should have exclusive access
> + * to this struct and can free the skb inside it
> + */
> + kfree_skb(data->skb);
> + }
> +
I dont think for_each_present_cpu(cpu) is right
(I realize drop_monitor already uses this, but its a bug)
To use it, you must have a notifier to react to cpu HOTPLUG events.
-> for_each_possible_cpu() is more correct.
Also, please dont add new printk(KERN_WARNING ...), use pr_warn(...)
instead
^ permalink raw reply
* [PATCH] netfilter: xt_HMARK: fix endian bugs and warnings
From: Hans Schillstrom @ 2012-05-16 15:00 UTC (permalink / raw)
To: pablo, kaber, jengelh, netfilter-devel, netdev, dan.carpenter
Cc: hans, Hans Schillstrom
A mix of u32 and __be32 causes endian warning.
The hash value produced is now the same for big and little endian machines.
i.e. a mix of Big and Little endian in a cluster is now possible.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
---
include/linux/netfilter/xt_HMARK.h | 5 ++-
net/netfilter/xt_HMARK.c | 68 ++++++++++++++++++++---------------
2 files changed, 42 insertions(+), 31 deletions(-)
diff --git a/include/linux/netfilter/xt_HMARK.h b/include/linux/netfilter/xt_HMARK.h
index abb1650..8b6307a 100644
--- a/include/linux/netfilter/xt_HMARK.h
+++ b/include/linux/netfilter/xt_HMARK.h
@@ -24,10 +24,11 @@ enum {
union hmark_ports {
struct {
- __u16 src;
- __u16 dst;
+ __be16 src;
+ __be16 dst;
} p16;
__u32 v32;
+ __be32 b32;
};
struct xt_hmark_info {
diff --git a/net/netfilter/xt_HMARK.c b/net/netfilter/xt_HMARK.c
index 32fbd73..7bb7b5e 100644
--- a/net/netfilter/xt_HMARK.c
+++ b/net/netfilter/xt_HMARK.c
@@ -32,13 +32,13 @@ MODULE_ALIAS("ipt_HMARK");
MODULE_ALIAS("ip6t_HMARK");
struct hmark_tuple {
- u32 src;
- u32 dst;
+ __be32 src;
+ __be32 dst;
union hmark_ports uports;
uint8_t proto;
};
-static inline u32 hmark_addr6_mask(const __u32 *addr32, const __u32 *mask)
+static inline __be32 hmark_addr6_mask(const __be32 *addr32, const __be32 *mask)
{
return (addr32[0] & mask[0]) ^
(addr32[1] & mask[1]) ^
@@ -46,8 +46,8 @@ static inline u32 hmark_addr6_mask(const __u32 *addr32, const __u32 *mask)
(addr32[3] & mask[3]);
}
-static inline u32
-hmark_addr_mask(int l3num, const __u32 *addr32, const __u32 *mask)
+static inline __be32
+hmark_addr_mask(int l3num, const __be32 *addr32, const __be32 *mask)
{
switch (l3num) {
case AF_INET:
@@ -58,6 +58,25 @@ hmark_addr_mask(int l3num, const __u32 *addr32, const __u32 *mask)
return 0;
}
+static inline void hmark_port_order(union hmark_ports *uports,
+ const struct xt_hmark_info *info)
+{
+ union hmark_ports hp;
+
+ hp.b32 = (uports->b32 & info->port_mask.b32) | info->port_set.b32;
+ hp.v32 = ntohl(hp.b32);
+ /* Make it endian safe into jhash() */
+#if defined(__LITTLE_ENDIAN)
+ if ((__force u16) uports->p16.dst >
+ (__force u16) uports->p16.src)
+#else
+ if ((__force u16) uports->p16.src >
+ (__force u16) uports->p16.dst)
+#endif
+ swap(hp.p16.src, hp.p16.dst);
+ uports->v32 = hp.v32;
+}
+
static int
hmark_ct_set_htuple(const struct sk_buff *skb, struct hmark_tuple *t,
const struct xt_hmark_info *info)
@@ -74,10 +93,10 @@ hmark_ct_set_htuple(const struct sk_buff *skb, struct hmark_tuple *t,
otuple = &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple;
rtuple = &ct->tuplehash[IP_CT_DIR_REPLY].tuple;
- t->src = hmark_addr_mask(otuple->src.l3num, otuple->src.u3.all,
- info->src_mask.all);
- t->dst = hmark_addr_mask(otuple->src.l3num, rtuple->src.u3.all,
- info->dst_mask.all);
+ t->src = hmark_addr_mask(otuple->src.l3num, otuple->src.u3.ip6,
+ info->src_mask.ip6);
+ t->dst = hmark_addr_mask(otuple->src.l3num, rtuple->src.u3.ip6,
+ info->dst_mask.ip6);
if (info->flags & XT_HMARK_FLAG(XT_HMARK_METHOD_L3))
return 0;
@@ -86,10 +105,7 @@ hmark_ct_set_htuple(const struct sk_buff *skb, struct hmark_tuple *t,
if (t->proto != IPPROTO_ICMP) {
t->uports.p16.src = otuple->src.u.all;
t->uports.p16.dst = rtuple->src.u.all;
- t->uports.v32 = (t->uports.v32 & info->port_mask.v32) |
- info->port_set.v32;
- if (t->uports.p16.dst < t->uports.p16.src)
- swap(t->uports.p16.dst, t->uports.p16.src);
+ hmark_port_order(&t->uports, info);
}
return 0;
@@ -102,11 +118,13 @@ static inline u32
hmark_hash(struct hmark_tuple *t, const struct xt_hmark_info *info)
{
u32 hash;
+ u32 src = ntohl(t->src);
+ u32 dst = ntohl(t->dst);
- if (t->dst < t->src)
- swap(t->src, t->dst);
+ if (dst < src)
+ swap(src, dst);
- hash = jhash_3words(t->src, t->dst, t->uports.v32, info->hashrnd);
+ hash = jhash_3words(src, dst, t->uports.v32, info->hashrnd);
hash = hash ^ (t->proto & info->proto_mask);
return (((u64)hash * info->hmodulus) >> 32) + info->hoffset;
@@ -125,12 +143,7 @@ hmark_set_tuple_ports(const struct sk_buff *skb, unsigned int nhoff,
nhoff += protoff;
if (skb_copy_bits(skb, nhoff, &t->uports, sizeof(t->uports)) < 0)
return;
-
- t->uports.v32 = (t->uports.v32 & info->port_mask.v32) |
- info->port_set.v32;
-
- if (t->uports.p16.dst < t->uports.p16.src)
- swap(t->uports.p16.dst, t->uports.p16.src);
+ hmark_port_order(&t->uports, info);
}
#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
@@ -178,8 +191,8 @@ hmark_pkt_set_htuple_ipv6(const struct sk_buff *skb, struct hmark_tuple *t,
return -1;
}
noicmp:
- t->src = hmark_addr6_mask(ip6->saddr.s6_addr32, info->src_mask.all);
- t->dst = hmark_addr6_mask(ip6->daddr.s6_addr32, info->dst_mask.all);
+ t->src = hmark_addr6_mask(ip6->saddr.s6_addr32, info->src_mask.ip6);
+ t->dst = hmark_addr6_mask(ip6->daddr.s6_addr32, info->dst_mask.ip6);
if (info->flags & XT_HMARK_FLAG(XT_HMARK_METHOD_L3))
return 0;
@@ -255,11 +268,8 @@ hmark_pkt_set_htuple_ipv4(const struct sk_buff *skb, struct hmark_tuple *t,
}
}
- t->src = (__force u32) ip->saddr;
- t->dst = (__force u32) ip->daddr;
-
- t->src &= info->src_mask.ip;
- t->dst &= info->dst_mask.ip;
+ t->src = ip->saddr & info->src_mask.ip;
+ t->dst = ip->daddr & info->dst_mask.ip;
if (info->flags & XT_HMARK_FLAG(XT_HMARK_METHOD_L3))
return 0;
--
1.7.2.3
^ permalink raw reply related
* Re: [PATCH] drop_monitor: convert to modular building
From: Ben Hutchings @ 2012-05-16 15:01 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, David S. Miller
In-Reply-To: <1337178426-2470-1-git-send-email-nhorman@tuxdriver.com>
On Wed, 2012-05-16 at 10:27 -0400, Neil Horman wrote:
> When I first wrote drop monitor I wrote it to just build monolithically. There
> is no reason it can't be built modularly as well, so lets give it that
> flexibiity.
Yes, please.
[...]
> --- a/net/core/drop_monitor.c
> +++ b/net/core/drop_monitor.c
> @@ -22,6 +22,7 @@
> #include <linux/timer.h>
> #include <linux/bitops.h>
> #include <linux/slab.h>
> +#include <linux/module.h>
> #include <net/genetlink.h>
> #include <net/netevent.h>
>
> @@ -223,9 +224,15 @@ static int set_all_monitor_traces(int state)
>
> switch (state) {
> case TRACE_ON:
> + if (!try_module_get(THIS_MODULE)) {
> + rc = -EINVAL;
Minor issue, but this isn't the right error code - there is nothing
invalid about the request, it just came at the wrong time. Perhaps
ENODEV or ECANCELED?
> + break;
> + }
> +
> rc |= register_trace_kfree_skb(trace_kfree_skb_hit, NULL);
> rc |= register_trace_napi_poll(trace_napi_poll_hit, NULL);
> break;
> +
> case TRACE_OFF:
> rc |= unregister_trace_kfree_skb(trace_kfree_skb_hit, NULL);
> rc |= unregister_trace_napi_poll(trace_napi_poll_hit, NULL);
> @@ -241,6 +248,9 @@ static int set_all_monitor_traces(int state)
> kfree_rcu(new_stat, rcu);
> }
> }
> +
> + module_put(THIS_MODULE);
> +
> break;
> default:
> rc = 1;
> @@ -383,4 +393,38 @@ out:
> return rc;
> }
>
> -late_initcall(init_net_drop_monitor);
> +static void exit_net_drop_monitor(void)
> +{
> + struct per_cpu_dm_data *data;
> + int cpu;
> +
> + if (unregister_netdevice_notifier(&dropmon_net_notifier))
> + printk(KERN_WARNING "Unable to unregiser dropmon notifer\n");
Currently this will only fail if you didn't actually register the
notifier, which would be a bug. If there is ever any other reason this
could fail, continuing with the notifier still registered would be
disastrous. Therefore I think this should be:
rc = unregister_netdevice_notifier(&dropmon_net_notifier);
BUG_ON(rc);
> + /*
> + * Because of the module_get/put we do in the trace state change path
> + * we are guarnateed not to have any current users when we get here
> + * all we need to do is make sure that we don't have any running timers
> + * or pending schedule calls
> + */
Surely you need to call set_all_monitor_traces(TRACE_OFF) first...
> + for_each_present_cpu(cpu) {
> + data = &per_cpu(dm_cpu_data, cpu);
> + del_timer(&data->send_timer);
> + cancel_work_sync(&data->dm_alert_work);
> + /*
> + * At this point, we should have exclusive access
> + * to this struct and can free the skb inside it
> + */
> + kfree_skb(data->skb);
> + }
> +
> + if (genl_unregister_family(&net_drop_monitor_family))
> + printk(KERN_WARNING "Unable to unregister drop monitor socket family\n");
Same issue as with unregister_netdevice_notifier().
Ben.
> +}
> +
> +module_init(init_net_drop_monitor);
> +module_exit(exit_net_drop_monitor);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Neil Horman <nhorman@tuxdriver.com>");
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [V2 PATCH 2/9] macvtap: zerocopy: fix truesize underestimation
From: Shirley Ma @ 2012-05-16 15:03 UTC (permalink / raw)
To: Jason Wang; +Cc: eric.dumazet, mst, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <4FB3192E.4030803@redhat.com>
On Wed, 2012-05-16 at 11:04 +0800, Jason Wang wrote:
> >> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> >> index bd4a70d..7cb2684 100644
> >> --- a/drivers/net/macvtap.c
> >> +++ b/drivers/net/macvtap.c
> >> @@ -519,6 +519,7 @@ static int zerocopy_sg_from_iovec(struct
> sk_buff
> >> *skb, const struct iovec *from,
> >> struct page *page[MAX_SKB_FRAGS];
> >> int num_pages;
> >> unsigned long base;
> >> + unsigned long truesize;
> >>
> >> len = from->iov_len - offset;
> >> if (!len) {
> >> @@ -533,10 +534,11 @@ static int zerocopy_sg_from_iovec(struct
> sk_buff
> >> *skb, const struct iovec *from,
> >> (num_pages> MAX_SKB_FRAGS -
> >> skb_shinfo(skb)->nr_frags))
> >> /* put_page is in skb free */
> >> return -EFAULT;
> >> + truesize = size * PAGE_SIZE;
> > Here should be truesize = size * PAGE_SIZE - offset, right?
> >
>
> We get the whole user page, so need to account them all. Also this is
> aligned with skb_copy_ubufs().
Then this would double count the size of "first" offset left from
previous copy, both skb->len and truesize.
Thanks
Shirley
^ permalink raw reply
* Re: [V2 PATCH 9/9] vhost: zerocopy: poll vq in zerocopy callback
From: Shirley Ma @ 2012-05-16 15:10 UTC (permalink / raw)
To: Jason Wang; +Cc: eric.dumazet, mst, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <4FB317C8.90002@redhat.com>
On Wed, 2012-05-16 at 10:58 +0800, Jason Wang wrote:
> >> drivers/vhost/vhost.c | 1 +
> >> 1 files changed, 1 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> >> index 947f00d..7b75fdf 100644
> >> --- a/drivers/vhost/vhost.c
> >> +++ b/drivers/vhost/vhost.c
> >> @@ -1604,6 +1604,7 @@ void vhost_zerocopy_callback(void *arg)
> >> struct vhost_ubuf_ref *ubufs = ubuf->arg;
> >> struct vhost_virtqueue *vq = ubufs->vq;
> >>
> >> + vhost_poll_queue(&vq->poll);
> >> /* set len = 1 to mark this desc buffers done DMA */
> >> vq->heads[ubuf->desc].len = VHOST_DMA_DONE_LEN;
> >> kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
> > Doing so, we might have redundant vhost_poll_queue(). Do you know in
> > which scenario there might be missing of adding and signaling during
> > zerocopy?
>
> Yes, as we only do signaling and adding during tx work, if there's no
> tx
> work when the skb were sent, we may lose the opportunity to let guest
> know about the completion. It's easy to be reproduced with netperf
> test.
The reason which host signals guest is to free guest tx buffers, if
there is no tx work, then it's not necessary to signal the guest unless
guest runs out of memory. The pending buffers will be released
virtio_net device gone.
What's the behavior of netperf test when you hit this situation?
Thanks
Shirley
^ permalink raw reply
* Re: [V2 PATCH 9/9] vhost: zerocopy: poll vq in zerocopy callback
From: Michael S. Tsirkin @ 2012-05-16 15:14 UTC (permalink / raw)
To: Shirley Ma
Cc: Jason Wang, eric.dumazet, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <1337181027.10741.13.camel@oc3660625478.ibm.com>
On Wed, May 16, 2012 at 08:10:27AM -0700, Shirley Ma wrote:
> On Wed, 2012-05-16 at 10:58 +0800, Jason Wang wrote:
> > >> drivers/vhost/vhost.c | 1 +
> > >> 1 files changed, 1 insertions(+), 0 deletions(-)
> > >>
> > >> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > >> index 947f00d..7b75fdf 100644
> > >> --- a/drivers/vhost/vhost.c
> > >> +++ b/drivers/vhost/vhost.c
> > >> @@ -1604,6 +1604,7 @@ void vhost_zerocopy_callback(void *arg)
> > >> struct vhost_ubuf_ref *ubufs = ubuf->arg;
> > >> struct vhost_virtqueue *vq = ubufs->vq;
> > >>
> > >> + vhost_poll_queue(&vq->poll);
> > >> /* set len = 1 to mark this desc buffers done DMA */
> > >> vq->heads[ubuf->desc].len = VHOST_DMA_DONE_LEN;
> > >> kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
> > > Doing so, we might have redundant vhost_poll_queue(). Do you know in
> > > which scenario there might be missing of adding and signaling during
> > > zerocopy?
> >
> > Yes, as we only do signaling and adding during tx work, if there's no
> > tx
> > work when the skb were sent, we may lose the opportunity to let guest
> > know about the completion. It's easy to be reproduced with netperf
> > test.
>
> The reason which host signals guest is to free guest tx buffers, if
> there is no tx work, then it's not necessary to signal the guest unless
> guest runs out of memory. The pending buffers will be released
> virtio_net device gone.
>
> What's the behavior of netperf test when you hit this situation?
>
> Thanks
> Shirley
IIRC guest networking seems to be lost.
--
MST
^ permalink raw reply
* Re: [PATCH] drop_monitor: convert to modular building
From: Neil Horman @ 2012-05-16 15:16 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, David S. Miller
In-Reply-To: <1337179681.8512.1212.camel@edumazet-glaptop>
On Wed, May 16, 2012 at 04:48:01PM +0200, Eric Dumazet wrote:
> On Wed, 2012-05-16 at 10:27 -0400, Neil Horman wrote:
> > When I first wrote drop monitor I wrote it to just build monolithically. There
> > is no reason it can't be built modularly as well, so lets give it that
> > flexibiity.
>
> > + for_each_present_cpu(cpu) {
> > + data = &per_cpu(dm_cpu_data, cpu);
> > + del_timer(&data->send_timer);
> > + cancel_work_sync(&data->dm_alert_work);
> > + /*
> > + * At this point, we should have exclusive access
> > + * to this struct and can free the skb inside it
> > + */
> > + kfree_skb(data->skb);
> > + }
> > +
>
> I dont think for_each_present_cpu(cpu) is right
>
> (I realize drop_monitor already uses this, but its a bug)
>
> To use it, you must have a notifier to react to cpu HOTPLUG events.
>
> -> for_each_possible_cpu() is more correct.
>
Ok, i can do that.
> Also, please dont add new printk(KERN_WARNING ...), use pr_warn(...)
> instead
>
Ack, I'll add a patch to this series to convert the existing printks to their
corresponding pr_* macros
Neil
>
>
>
^ permalink raw reply
* Re: [PATCH RFC] tun: experimental zero copy tx support
From: Shirley Ma @ 2012-05-16 15:16 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David S. Miller, Stephen Hemminger, Joe Perches, Jason Wang,
netdev, linux-kernel, Ian.Campbell, kvm
In-Reply-To: <20120514183929.GB17086@redhat.com>
On Mon, 2012-05-14 at 21:39 +0300, Michael S. Tsirkin wrote:
> > Hello Mike,
> >
> > Have you tested this patch? I think the difference between macvtap
> and
> > tap is tap forwarding the packet to bridge. The zerocopy is disabled
> in
> > this case.
> >
> > Shirley
>
> Testing in progress, but the patchset I pointed to enables
> zerocopy with bridge.
Hello Mike,
You meant this patch or another patchset for enabling bridge zerocopy?
I remembered we disabled forward skb zerocopy since the user space
program might hold the buffers too long or forever.
In tap/bridge case, when the tx buffers will be released?
Thanks
Shirley
^ permalink raw reply
* Re: [PATCH] drop_monitor: convert to modular building
From: Neil Horman @ 2012-05-16 15:19 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, David S. Miller
In-Reply-To: <1337180505.2568.10.camel@bwh-desktop.uk.solarflarecom.com>
On Wed, May 16, 2012 at 04:01:45PM +0100, Ben Hutchings wrote:
> On Wed, 2012-05-16 at 10:27 -0400, Neil Horman wrote:
> > When I first wrote drop monitor I wrote it to just build monolithically. There
> > is no reason it can't be built modularly as well, so lets give it that
> > flexibiity.
>
> Yes, please.
>
> [...]
> > --- a/net/core/drop_monitor.c
> > +++ b/net/core/drop_monitor.c
> > @@ -22,6 +22,7 @@
> > #include <linux/timer.h>
> > #include <linux/bitops.h>
> > #include <linux/slab.h>
> > +#include <linux/module.h>
> > #include <net/genetlink.h>
> > #include <net/netevent.h>
> >
> > @@ -223,9 +224,15 @@ static int set_all_monitor_traces(int state)
> >
> > switch (state) {
> > case TRACE_ON:
> > + if (!try_module_get(THIS_MODULE)) {
> > + rc = -EINVAL;
>
> Minor issue, but this isn't the right error code - there is nothing
> invalid about the request, it just came at the wrong time. Perhaps
> ENODEV or ECANCELED?
>
Yeah, ok, ENODEV seems reasonable.
> > + break;
> > + }
> > +
> > rc |= register_trace_kfree_skb(trace_kfree_skb_hit, NULL);
> > rc |= register_trace_napi_poll(trace_napi_poll_hit, NULL);
> > break;
> > +
> > case TRACE_OFF:
> > rc |= unregister_trace_kfree_skb(trace_kfree_skb_hit, NULL);
> > rc |= unregister_trace_napi_poll(trace_napi_poll_hit, NULL);
> > @@ -241,6 +248,9 @@ static int set_all_monitor_traces(int state)
> > kfree_rcu(new_stat, rcu);
> > }
> > }
> > +
> > + module_put(THIS_MODULE);
> > +
> > break;
> > default:
> > rc = 1;
> > @@ -383,4 +393,38 @@ out:
> > return rc;
> > }
> >
> > -late_initcall(init_net_drop_monitor);
> > +static void exit_net_drop_monitor(void)
> > +{
> > + struct per_cpu_dm_data *data;
> > + int cpu;
> > +
> > + if (unregister_netdevice_notifier(&dropmon_net_notifier))
> > + printk(KERN_WARNING "Unable to unregiser dropmon notifer\n");
>
> Currently this will only fail if you didn't actually register the
> notifier, which would be a bug. If there is ever any other reason this
> could fail, continuing with the notifier still registered would be
> disastrous. Therefore I think this should be:
>
> rc = unregister_netdevice_notifier(&dropmon_net_notifier);
> BUG_ON(rc);
>
Ok, seems reasonable.
> > + /*
> > + * Because of the module_get/put we do in the trace state change path
> > + * we are guarnateed not to have any current users when we get here
> > + * all we need to do is make sure that we don't have any running timers
> > + * or pending schedule calls
> > + */
>
> Surely you need to call set_all_monitor_traces(TRACE_OFF) first...
>
Nope, If you'll note the code higher up in the patch, I use try_module_get and
module_put to prevent the module unload code from getting here while anyone is
actually using the protocol. Once we are in the module remove routine here, we
are guaranateed that there are no users of this protocol, and that the
tracepoints are all unregistered.
> > + for_each_present_cpu(cpu) {
> > + data = &per_cpu(dm_cpu_data, cpu);
> > + del_timer(&data->send_timer);
> > + cancel_work_sync(&data->dm_alert_work);
> > + /*
> > + * At this point, we should have exclusive access
> > + * to this struct and can free the skb inside it
> > + */
> > + kfree_skb(data->skb);
> > + }
> > +
> > + if (genl_unregister_family(&net_drop_monitor_family))
> > + printk(KERN_WARNING "Unable to unregister drop monitor socket family\n");
>
> Same issue as with unregister_netdevice_notifier().
>
ack, I'll update that to a BUG_ON
Neil
^ permalink raw reply
* Re: [PATCH 0/3] net: mac80211: Neaten debugging
From: Joe Perches @ 2012-05-16 15:22 UTC (permalink / raw)
To: Johannes Berg
Cc: David Miller, linville, linux-wireless, netdev, linux-kernel
In-Reply-To: <1337155171.4367.2.camel@jlt3.sipsolutions.net>
On Wed, 2012-05-16 at 09:59 +0200, Johannes Berg wrote:
> I wonder if it makes sense to leave these under
> #ifdef though? Why #ifdef something if it's going to be invisible most
> of the time anyway?
I don't understand your point.
#ifdef removal is a good thing.
^ permalink raw reply
* Re: [PATCH 0/3] net: mac80211: Neaten debugging
From: Johannes Berg @ 2012-05-16 15:30 UTC (permalink / raw)
To: Joe Perches; +Cc: David Miller, linville, linux-wireless, netdev, linux-kernel
In-Reply-To: <1337181771.4818.0.camel@joe2Laptop>
On Wed, 2012-05-16 at 08:22 -0700, Joe Perches wrote:
> On Wed, 2012-05-16 at 09:59 +0200, Johannes Berg wrote:
> > I wonder if it makes sense to leave these under
> > #ifdef though? Why #ifdef something if it's going to be invisible most
> > of the time anyway?
>
> I don't understand your point.
> #ifdef removal is a good thing.
Yeah, but you left a lot of them under ifdef, and I'm wondering why you
didn't remove them, or if you should, or ...
johannes
^ permalink raw reply
* Re: [PATCH] drop_monitor: convert to modular building
From: Ben Hutchings @ 2012-05-16 15:34 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, David S. Miller
In-Reply-To: <20120516151932.GB30195@hmsreliant.think-freely.org>
On Wed, 2012-05-16 at 11:19 -0400, Neil Horman wrote:
> On Wed, May 16, 2012 at 04:01:45PM +0100, Ben Hutchings wrote:
> > On Wed, 2012-05-16 at 10:27 -0400, Neil Horman wrote:
> > > When I first wrote drop monitor I wrote it to just build monolithically. There
> > > is no reason it can't be built modularly as well, so lets give it that
> > > flexibiity.
[...]
> > > + /*
> > > + * Because of the module_get/put we do in the trace state change path
> > > + * we are guarnateed not to have any current users when we get here
> > > + * all we need to do is make sure that we don't have any running timers
> > > + * or pending schedule calls
> > > + */
> >
> > Surely you need to call set_all_monitor_traces(TRACE_OFF) first...
> >
> Nope, If you'll note the code higher up in the patch, I use try_module_get and
> module_put to prevent the module unload code from getting here while anyone is
> actually using the protocol. Once we are in the module remove routine here, we
> are guaranateed that there are no users of this protocol, and that the
> tracepoints are all unregistered.
[...]
Yes, of course.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH 0/3] net: mac80211: Neaten debugging
From: Joe Perches @ 2012-05-16 15:43 UTC (permalink / raw)
To: Johannes Berg
Cc: David Miller, linville, linux-wireless, netdev, linux-kernel
In-Reply-To: <1337182220.18519.5.camel@jlt3.sipsolutions.net>
On Wed, 2012-05-16 at 17:30 +0200, Johannes Berg wrote:
> On Wed, 2012-05-16 at 08:22 -0700, Joe Perches wrote:
> > On Wed, 2012-05-16 at 09:59 +0200, Johannes Berg wrote:
> > > I wonder if it makes sense to leave these under
> > > #ifdef though? Why #ifdef something if it's going to be invisible most
> > > of the time anyway?
> >
> > I don't understand your point.
> > #ifdef removal is a good thing.
>
> Yeah, but you left a lot of them under ifdef, and I'm wondering why you
> didn't remove them, or if you should, or ...
Those mostly use other different
#ifdef CONFIG_SOME_OTHER_CONTROL elements.
There are I think 3 that I left because they do
not use printk/pr_level but use wiphy_<level> or
netdev_<level>.
^ permalink raw reply
* Re: [PATCH 0/3] net: mac80211: Neaten debugging
From: Johannes Berg @ 2012-05-16 15:56 UTC (permalink / raw)
To: Joe Perches; +Cc: David Miller, linville, linux-wireless, netdev, linux-kernel
In-Reply-To: <1337183004.4818.2.camel@joe2Laptop>
On Wed, 2012-05-16 at 08:43 -0700, Joe Perches wrote:
> On Wed, 2012-05-16 at 17:30 +0200, Johannes Berg wrote:
> > On Wed, 2012-05-16 at 08:22 -0700, Joe Perches wrote:
> > > On Wed, 2012-05-16 at 09:59 +0200, Johannes Berg wrote:
> > > > I wonder if it makes sense to leave these under
> > > > #ifdef though? Why #ifdef something if it's going to be invisible most
> > > > of the time anyway?
> > >
> > > I don't understand your point.
> > > #ifdef removal is a good thing.
> >
> > Yeah, but you left a lot of them under ifdef, and I'm wondering why you
> > didn't remove them, or if you should, or ...
>
> Those mostly use other different
> #ifdef CONFIG_SOME_OTHER_CONTROL elements.
>
> There are I think 3 that I left because they do
> not use printk/pr_level but use wiphy_<level> or
> netdev_<level>.
Hmm, ok. I guess I need to just look at them and decide what should be
what. Thanks for the preparation work though :)
johannes
^ permalink raw reply
* [PATCH net-next] net: sock_flag() cleanup
From: Eric Dumazet @ 2012-05-16 15:57 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
- sock_flag() accepts a const pointer
- sock_flag() returns a boolean
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/sock.h | 2 +-
net/core/sock.c | 14 +++++++-------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index e613704..036f506 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -650,7 +650,7 @@ static inline void sock_reset_flag(struct sock *sk, enum sock_flags flag)
__clear_bit(flag, &sk->sk_flags);
}
-static inline int sock_flag(struct sock *sk, enum sock_flags flag)
+static inline bool sock_flag(const struct sock *sk, enum sock_flags flag)
{
return test_bit(flag, &sk->sk_flags);
}
diff --git a/net/core/sock.c b/net/core/sock.c
index 26ed27f..9d144ee 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -849,7 +849,7 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
break;
case SO_BROADCAST:
- v.val = !!sock_flag(sk, SOCK_BROADCAST);
+ v.val = sock_flag(sk, SOCK_BROADCAST);
break;
case SO_SNDBUF:
@@ -865,7 +865,7 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
break;
case SO_KEEPALIVE:
- v.val = !!sock_flag(sk, SOCK_KEEPOPEN);
+ v.val = sock_flag(sk, SOCK_KEEPOPEN);
break;
case SO_TYPE:
@@ -887,7 +887,7 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
break;
case SO_OOBINLINE:
- v.val = !!sock_flag(sk, SOCK_URGINLINE);
+ v.val = sock_flag(sk, SOCK_URGINLINE);
break;
case SO_NO_CHECK:
@@ -900,7 +900,7 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
case SO_LINGER:
lv = sizeof(v.ling);
- v.ling.l_onoff = !!sock_flag(sk, SOCK_LINGER);
+ v.ling.l_onoff = sock_flag(sk, SOCK_LINGER);
v.ling.l_linger = sk->sk_lingertime / HZ;
break;
@@ -1012,11 +1012,11 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
break;
case SO_RXQ_OVFL:
- v.val = !!sock_flag(sk, SOCK_RXQ_OVFL);
+ v.val = sock_flag(sk, SOCK_RXQ_OVFL);
break;
case SO_WIFI_STATUS:
- v.val = !!sock_flag(sk, SOCK_WIFI_STATUS);
+ v.val = sock_flag(sk, SOCK_WIFI_STATUS);
break;
case SO_PEEK_OFF:
@@ -1026,7 +1026,7 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
v.val = sk->sk_peek_off;
break;
case SO_NOFCS:
- v.val = !!sock_flag(sk, SOCK_NOFCS);
+ v.val = sock_flag(sk, SOCK_NOFCS);
break;
default:
return -ENOPROTOOPT;
^ permalink raw reply related
* Re: [PATCH RFC] tun: experimental zero copy tx support
From: Michael S. Tsirkin @ 2012-05-16 16:51 UTC (permalink / raw)
To: Shirley Ma
Cc: David S. Miller, Stephen Hemminger, Joe Perches, Jason Wang,
netdev, linux-kernel, Ian.Campbell, kvm
In-Reply-To: <1337181415.10741.18.camel@oc3660625478.ibm.com>
On Wed, May 16, 2012 at 08:16:55AM -0700, Shirley Ma wrote:
> On Mon, 2012-05-14 at 21:39 +0300, Michael S. Tsirkin wrote:
> > > Hello Mike,
> > >
> > > Have you tested this patch? I think the difference between macvtap
> > and
> > > tap is tap forwarding the packet to bridge. The zerocopy is disabled
> > in
> > > this case.
> > >
> > > Shirley
> >
> > Testing in progress, but the patchset I pointed to enables
> > zerocopy with bridge.
>
> Hello Mike,
>
> You meant this patch or another patchset for enabling bridge zerocopy?
>
> I remembered we disabled forward skb zerocopy since the user space
> program might hold the buffers too long or forever.
>
> In tap/bridge case, when the tx buffers will be released?
>
> Thanks
> Shirley
It still fails some tests for me but maybe I'll post the whole
patchset so you can see how it works.
^ permalink raw reply
* Re: [PATCH] ptp_pch: Add missing #include <linux/slab.h>
From: Richard Cochran @ 2012-05-16 17:19 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Takahiro Shimizu, David S. Miller, netdev, linux-kernel
In-Reply-To: <1337169017-15835-1-git-send-email-geert@linux-m68k.org>
On Wed, May 16, 2012 at 01:50:17PM +0200, Geert Uytterhoeven wrote:
> drivers/ptp/ptp_pch.c: In function 'pch_remove':
> drivers/ptp/ptp_pch.c:576:2: error: implicit declaration of function 'kfree' [-Werror=implicit-function-declaration]
> drivers/ptp/ptp_pch.c: In function 'pch_probe':
> drivers/ptp/ptp_pch.c:587:2: error: implicit declaration of function 'kzalloc' [-Werror=implicit-function-declaration]
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Acked-by: Richard Cochran <richardcochran@gmail.com>
Thanks,
Richard
> ---
> Not even compile-tested, as it fails on parisc only.
> http://kisskb.ellerman.id.au/kisskb/buildresult/6312813/
>
> drivers/ptp/ptp_pch.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/ptp/ptp_pch.c b/drivers/ptp/ptp_pch.c
> index 375eb04..6fff680 100644
> --- a/drivers/ptp/ptp_pch.c
> +++ b/drivers/ptp/ptp_pch.c
> @@ -30,6 +30,7 @@
> #include <linux/module.h>
> #include <linux/pci.h>
> #include <linux/ptp_clock_kernel.h>
> +#include <linux/slab.h>
>
> #define STATION_ADDR_LEN 20
> #define PCI_DEVICE_ID_PCH_1588 0x8819
> --
> 1.7.0.4
>
^ permalink raw reply
* Re: [V2 PATCH 9/9] vhost: zerocopy: poll vq in zerocopy callback
From: Shirley Ma @ 2012-05-16 17:32 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, eric.dumazet, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <20120516151444.GC9934@redhat.com>
On Wed, 2012-05-16 at 18:14 +0300, Michael S. Tsirkin wrote:
> On Wed, May 16, 2012 at 08:10:27AM -0700, Shirley Ma wrote:
> > On Wed, 2012-05-16 at 10:58 +0800, Jason Wang wrote:
> > > >> drivers/vhost/vhost.c | 1 +
> > > >> 1 files changed, 1 insertions(+), 0 deletions(-)
> > > >>
> > > >> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > >> index 947f00d..7b75fdf 100644
> > > >> --- a/drivers/vhost/vhost.c
> > > >> +++ b/drivers/vhost/vhost.c
> > > >> @@ -1604,6 +1604,7 @@ void vhost_zerocopy_callback(void *arg)
> > > >> struct vhost_ubuf_ref *ubufs = ubuf->arg;
> > > >> struct vhost_virtqueue *vq = ubufs->vq;
> > > >>
> > > >> + vhost_poll_queue(&vq->poll);
> > > >> /* set len = 1 to mark this desc buffers done DMA */
> > > >> vq->heads[ubuf->desc].len = VHOST_DMA_DONE_LEN;
> > > >> kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
> > > > Doing so, we might have redundant vhost_poll_queue(). Do you
> know in
> > > > which scenario there might be missing of adding and signaling
> during
> > > > zerocopy?
> > >
> > > Yes, as we only do signaling and adding during tx work, if there's
> no
> > > tx
> > > work when the skb were sent, we may lose the opportunity to let
> guest
> > > know about the completion. It's easy to be reproduced with netperf
> > > test.
> >
> > The reason which host signals guest is to free guest tx buffers, if
> > there is no tx work, then it's not necessary to signal the guest
> unless
> > guest runs out of memory. The pending buffers will be released
> > virtio_net device gone.
> >
> > What's the behavior of netperf test when you hit this situation?
> >
> > Thanks
> > Shirley
>
> IIRC guest networking seems to be lost.
It seems vhost_enable_notify is missing in somewhere else?
Thanks
Shirley
^ permalink raw reply
* Re: [V2 PATCH 9/9] vhost: zerocopy: poll vq in zerocopy callback
From: Michael S. Tsirkin @ 2012-05-16 18:36 UTC (permalink / raw)
To: Shirley Ma
Cc: Jason Wang, eric.dumazet, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <1337189525.10741.24.camel@oc3660625478.ibm.com>
On Wed, May 16, 2012 at 10:32:05AM -0700, Shirley Ma wrote:
> On Wed, 2012-05-16 at 18:14 +0300, Michael S. Tsirkin wrote:
> > On Wed, May 16, 2012 at 08:10:27AM -0700, Shirley Ma wrote:
> > > On Wed, 2012-05-16 at 10:58 +0800, Jason Wang wrote:
> > > > >> drivers/vhost/vhost.c | 1 +
> > > > >> 1 files changed, 1 insertions(+), 0 deletions(-)
> > > > >>
> > > > >> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > > >> index 947f00d..7b75fdf 100644
> > > > >> --- a/drivers/vhost/vhost.c
> > > > >> +++ b/drivers/vhost/vhost.c
> > > > >> @@ -1604,6 +1604,7 @@ void vhost_zerocopy_callback(void *arg)
> > > > >> struct vhost_ubuf_ref *ubufs = ubuf->arg;
> > > > >> struct vhost_virtqueue *vq = ubufs->vq;
> > > > >>
> > > > >> + vhost_poll_queue(&vq->poll);
> > > > >> /* set len = 1 to mark this desc buffers done DMA */
> > > > >> vq->heads[ubuf->desc].len = VHOST_DMA_DONE_LEN;
> > > > >> kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
> > > > > Doing so, we might have redundant vhost_poll_queue(). Do you
> > know in
> > > > > which scenario there might be missing of adding and signaling
> > during
> > > > > zerocopy?
> > > >
> > > > Yes, as we only do signaling and adding during tx work, if there's
> > no
> > > > tx
> > > > work when the skb were sent, we may lose the opportunity to let
> > guest
> > > > know about the completion. It's easy to be reproduced with netperf
> > > > test.
> > >
> > > The reason which host signals guest is to free guest tx buffers, if
> > > there is no tx work, then it's not necessary to signal the guest
> > unless
> > > guest runs out of memory. The pending buffers will be released
> > > virtio_net device gone.
> > >
> > > What's the behavior of netperf test when you hit this situation?
> > >
> > > Thanks
> > > Shirley
> >
> > IIRC guest networking seems to be lost.
>
> It seems vhost_enable_notify is missing in somewhere else?
>
> Thanks
> Shirley
Donnu. I see virtio sending packets but they do not get
to tun on host. debugging.
^ permalink raw reply
* Re: [PATCH 0/4] netfilter fixes for 3.4-rc7
From: Jozsef Kadlecsik @ 2012-05-16 18:41 UTC (permalink / raw)
To: David Miller; +Cc: Pablo Neira Ayuso, netfilter-devel, netdev
In-Reply-To: <20120514.185607.1967456974676336550.davem@davemloft.net>
Hi Dave,
On Mon, 14 May 2012, David Miller wrote:
> From: pablo@netfilter.org
> Date: Mon, 14 May 2012 13:46:59 +0200
>
> > * One fix for possible timeout overflow for ipset, from Jozsef
> > Kadlecsik.
> >
> > * One fix to ensure that hash size is correct, again for ipset
> > from Jozsef Kadlecsik.
> >
> > * Removal of redundant include in xt_CT from Eldad Zack.
> >
> > * Fix for wrong usage of MODULE_ALIAS_NFCT_HELPER in nf_ct_h323
> > helper from myself.
>
> I don't consider any of these appropriate this late in the -RC
> series.
>
> They don't fix major regressions seen by many users.
Could at least the patch with the subject
netfilter: ipset: fix hash size checking in kernel
The hash size must fit both into u32 (jhash) and the max value of
size_t. The missing checking could lead to kernel crash, bug reported
by Seblu.
be submitted into 3.4-rc7? Any non most-recent ipset package compiled with
gcc-4.7 or above can trigger the bug.
Best regards,
Jozsef
-
E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
H-1525 Budapest 114, POB. 49, Hungary
^ permalink raw reply
* Re: [PATCH] ptp_pch: Add missing #include <linux/slab.h>
From: David Miller @ 2012-05-16 18:58 UTC (permalink / raw)
To: geert; +Cc: tshimizu818, richardcochran, netdev, linux-kernel
In-Reply-To: <1337169017-15835-1-git-send-email-geert@linux-m68k.org>
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Wed, 16 May 2012 13:50:17 +0200
> drivers/ptp/ptp_pch.c: In function 'pch_remove':
> drivers/ptp/ptp_pch.c:576:2: error: implicit declaration of function 'kfree' [-Werror=implicit-function-declaration]
> drivers/ptp/ptp_pch.c: In function 'pch_probe':
> drivers/ptp/ptp_pch.c:587:2: error: implicit declaration of function 'kzalloc' [-Werror=implicit-function-declaration]
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Applied.
^ 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