* Re: Wireless regression in workqueue: use mod_delayed_work() instead of __cancel + queue
From: Tejun Heo @ 2012-12-01 14:39 UTC (permalink / raw)
To: Anders Kaseorg
Cc: Herbert Xu, John W. Linville, netdev, linux-wireless,
linux-kernel
In-Reply-To: <alpine.DEB.2.00.1211302234410.25064@dr-wily.mit.edu>
Hey, Anders.
On Fri, Nov 30, 2012 at 11:15:50PM -0500, Anders Kaseorg wrote:
> Yes. I tested that both directly on top of the bad commit, and on
> v3.7-rc7, and it fixes the bug in both cases.
Can you please test this one too? Thanks!
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 042d221..94964d1 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1364,6 +1364,11 @@ static void __queue_delayed_work(int cpu, struct workqueue_struct *wq,
BUG_ON(timer_pending(timer));
BUG_ON(!list_empty(&work->entry));
+ if (!delay) {
+ __queue_work(cpu, wq, &dwork->work);
+ return;
+ }
+
timer_stats_timer_set_start_info(&dwork->timer);
/*
@@ -1417,9 +1422,6 @@ bool queue_delayed_work_on(int cpu, struct workqueue_struct *wq,
bool ret = false;
unsigned long flags;
- if (!delay)
- return queue_work_on(cpu, wq, &dwork->work);
-
/* read the comment in __queue_work() */
local_irq_save(flags);
^ permalink raw reply related
* [PATCH v4 net-next] sctp: Add support to per-association statistics via a new SCTP_GET_ASSOC_STATS call
From: Michele Baldessari @ 2012-12-01 14:49 UTC (permalink / raw)
To: linux-sctp
Cc: michele, Neil Horman, Thomas Graf, Vlad Yasevich, netdev,
David S. Miller
The current SCTP stack is lacking a mechanism to have per association
statistics. This is an implementation modeled after OpenSolaris'
SCTP_GET_ASSOC_STATS.
Userspace part will follow on lksctp if/when there is a general ACK on
this.
V4:
- Move ipackets++ before q->immediate.func() for consistency reasons
- Move sctp_max_rto() at the end of sctp_transport_update_rto() to avoid
returning bogus RTO values
- return asoc->rto_min when max_obs_rto value has not changed
V3:
- Increase ictrlchunks in sctp_assoc_bh_rcv() as well
- Move ipackets++ to sctp_inq_push()
- return 0 when no rto updates took place since the last call
V2:
- Implement partial retrieval of stat struct to cope for future expansion
- Kill the rtxpackets counter as it cannot be precise anyway
- Rename outseqtsns to outofseqtsns to make it clearer that these are out
of sequence unexpected TSNs
- Move asoc->ipackets++ under a lock to avoid potential miscounts
- Fold asoc->opackets++ into the already existing asoc check
- Kill unneeded (q->asoc) test when increasing rtxchunks
- Do not count octrlchunks if sending failed (SCTP_XMIT_OK != 0)
- Don't count SHUTDOWNs as SACKs
- Move SCTP_GET_ASSOC_STATS to the private space API
- Adjust the len check in sctp_getsockopt_assoc_stats() to allow for
future struct growth
- Move association statistics in their own struct
- Update idupchunks when we send a SACK with dup TSNs
- return min_rto in max_rto when RTO has not changed. Also return the
transport when max_rto last changed.
Signed-off: Michele Baldessari <michele@acksyn.org>
---
include/net/sctp/sctp.h | 12 ++++++++
include/net/sctp/structs.h | 36 ++++++++++++++++++++++++
include/net/sctp/user.h | 27 ++++++++++++++++++
net/sctp/associola.c | 10 ++++++-
net/sctp/endpointola.c | 5 +++-
net/sctp/inqueue.c | 2 ++
net/sctp/output.c | 14 ++++++----
net/sctp/outqueue.c | 12 ++++++--
net/sctp/sm_make_chunk.c | 5 ++--
net/sctp/sm_sideeffect.c | 1 +
net/sctp/sm_statefuns.c | 10 +++++--
net/sctp/socket.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++
net/sctp/transport.c | 2 ++
13 files changed, 192 insertions(+), 13 deletions(-)
diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 9c6414f..7fdf298 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -272,6 +272,18 @@ struct sctp_mib {
unsigned long mibs[SCTP_MIB_MAX];
};
+/* helper function to track stats about max rto and related transport */
+static inline void sctp_max_rto(struct sctp_association *asoc,
+ struct sctp_transport *trans)
+{
+ if (asoc->stats.max_obs_rto < (__u64)trans->rto) {
+ asoc->stats.max_obs_rto = trans->rto;
+ memset(&asoc->stats.obs_rto_ipaddr, 0,
+ sizeof(struct sockaddr_storage));
+ memcpy(&asoc->stats.obs_rto_ipaddr, &trans->ipaddr,
+ trans->af_specific->sockaddr_len);
+ }
+}
/* Print debugging messages. */
#if SCTP_DEBUG
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 2b2f61d..c252101 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1312,6 +1312,40 @@ struct sctp_inithdr_host {
__u32 initial_tsn;
};
+/* SCTP_GET_ASSOC_STATS counters */
+struct sctp_priv_assoc_stats {
+ /* Maximum observed rto in the association during subsequent
+ * observations. Value is set to 0 if no RTO measurement took place
+ * The transport where the max_rto was observed is returned in
+ * obs_rto_ipaddr
+ */
+ struct sockaddr_storage obs_rto_ipaddr;
+ __u64 max_obs_rto;
+ /* Total In and Out SACKs received and sent */
+ __u64 isacks;
+ __u64 osacks;
+ /* Total In and Out packets received and sent */
+ __u64 opackets;
+ __u64 ipackets;
+ /* Total retransmitted chunks */
+ __u64 rtxchunks;
+ /* TSN received > next expected */
+ __u64 outofseqtsns;
+ /* Duplicate Chunks received */
+ __u64 idupchunks;
+ /* Gap Ack Blocks received */
+ __u64 gapcnt;
+ /* Unordered data chunks sent and received */
+ __u64 ouodchunks;
+ __u64 iuodchunks;
+ /* Ordered data chunks sent and received */
+ __u64 oodchunks;
+ __u64 iodchunks;
+ /* Control chunks sent and received */
+ __u64 octrlchunks;
+ __u64 ictrlchunks;
+};
+
/* RFC2960
*
* 12. Recommended Transmission Control Block (TCB) Parameters
@@ -1830,6 +1864,8 @@ struct sctp_association {
__u8 need_ecne:1, /* Need to send an ECNE Chunk? */
temp:1; /* Is it a temporary association? */
+
+ struct sctp_priv_assoc_stats stats;
};
diff --git a/include/net/sctp/user.h b/include/net/sctp/user.h
index 1b02d7a..9a0ae09 100644
--- a/include/net/sctp/user.h
+++ b/include/net/sctp/user.h
@@ -107,6 +107,7 @@ typedef __s32 sctp_assoc_t;
#define SCTP_GET_LOCAL_ADDRS 109 /* Get all local address. */
#define SCTP_SOCKOPT_CONNECTX 110 /* CONNECTX requests. */
#define SCTP_SOCKOPT_CONNECTX3 111 /* CONNECTX requests (updated) */
+#define SCTP_GET_ASSOC_STATS 112 /* Read only */
/*
* 5.2.1 SCTP Initiation Structure (SCTP_INIT)
@@ -719,6 +720,32 @@ struct sctp_getaddrs {
__u8 addrs[0]; /*output, variable size*/
};
+/* A socket user request obtained via SCTP_GET_ASSOC_STATS that retrieves
+ * association stats. All stats are counts except sas_maxrto and
+ * sas_obs_rto_ipaddr. maxrto is the max observed rto + transport since
+ * the last call. Will return 0 when RTO was not update since last call
+ */
+struct sctp_assoc_stats {
+ sctp_assoc_t sas_assoc_id; /* Input */
+ /* Transport of observed max RTO */
+ struct sockaddr_storage sas_obs_rto_ipaddr;
+ __u64 sas_maxrto; /* Maximum Observed RTO for period */
+ __u64 sas_isacks; /* SACKs received */
+ __u64 sas_osacks; /* SACKs sent */
+ __u64 sas_opackets; /* Packets sent */
+ __u64 sas_ipackets; /* Packets received */
+ __u64 sas_rtxchunks; /* Retransmitted Chunks */
+ __u64 sas_outofseqtsns;/* TSN received > next expected */
+ __u64 sas_idupchunks; /* Dups received (ordered+unordered) */
+ __u64 sas_gapcnt; /* Gap Acknowledgements Received */
+ __u64 sas_ouodchunks; /* Unordered data chunks sent */
+ __u64 sas_iuodchunks; /* Unordered data chunks received */
+ __u64 sas_oodchunks; /* Ordered data chunks sent */
+ __u64 sas_iodchunks; /* Ordered data chunks received */
+ __u64 sas_octrlchunks; /* Control chunks sent */
+ __u64 sas_ictrlchunks; /* Control chunks received */
+};
+
/* These are bit fields for msghdr->msg_flags. See section 5.1. */
/* On user space Linux, these live in <bits/socket.h> as an enum. */
enum sctp_msg_flags {
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index b1ef3bc..ba3f9cc 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -321,6 +321,9 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
asoc->default_timetolive = sp->default_timetolive;
asoc->default_rcv_context = sp->default_rcv_context;
+ /* SCTP_GET_ASSOC_STATS COUNTERS */
+ memset(&asoc->stats, 0, sizeof(struct sctp_priv_assoc_stats));
+
/* AUTH related initializations */
INIT_LIST_HEAD(&asoc->endpoint_shared_keys);
err = sctp_auth_asoc_copy_shkeys(ep, asoc, gfp);
@@ -760,6 +763,7 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc,
/* Set the transport's RTO.initial value */
peer->rto = asoc->rto_initial;
+ sctp_max_rto(asoc, peer);
/* Set the peer's active state. */
peer->state = peer_state;
@@ -1152,8 +1156,12 @@ static void sctp_assoc_bh_rcv(struct work_struct *work)
*/
if (sctp_chunk_is_data(chunk))
asoc->peer.last_data_from = chunk->transport;
- else
+ else {
SCTP_INC_STATS(net, SCTP_MIB_INCTRLCHUNKS);
+ asoc->stats.ictrlchunks++;
+ if (chunk->chunk_hdr->type == SCTP_CID_SACK)
+ asoc->stats.isacks++;
+ }
if (chunk->transport)
chunk->transport->last_time_heard = jiffies;
diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index 1859e2b..32ab55b 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -480,8 +480,11 @@ normal:
*/
if (asoc && sctp_chunk_is_data(chunk))
asoc->peer.last_data_from = chunk->transport;
- else
+ else {
SCTP_INC_STATS(sock_net(ep->base.sk), SCTP_MIB_INCTRLCHUNKS);
+ if (asoc)
+ asoc->stats.ictrlchunks++;
+ }
if (chunk->transport)
chunk->transport->last_time_heard = jiffies;
diff --git a/net/sctp/inqueue.c b/net/sctp/inqueue.c
index 397296f..2d5ad28 100644
--- a/net/sctp/inqueue.c
+++ b/net/sctp/inqueue.c
@@ -104,6 +104,8 @@ void sctp_inq_push(struct sctp_inq *q, struct sctp_chunk *chunk)
* on the BH related data structures.
*/
list_add_tail(&chunk->list, &q->in_chunk_list);
+ if (chunk->asoc)
+ chunk->asoc->stats.ipackets++;
q->immediate.func(&q->immediate);
}
diff --git a/net/sctp/output.c b/net/sctp/output.c
index 4e90188bf..f5200a2 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -311,6 +311,8 @@ static sctp_xmit_t __sctp_packet_append_chunk(struct sctp_packet *packet,
case SCTP_CID_SACK:
packet->has_sack = 1;
+ if (chunk->asoc)
+ chunk->asoc->stats.osacks++;
break;
case SCTP_CID_AUTH:
@@ -584,11 +586,13 @@ int sctp_packet_transmit(struct sctp_packet *packet)
*/
/* Dump that on IP! */
- if (asoc && asoc->peer.last_sent_to != tp) {
- /* Considering the multiple CPU scenario, this is a
- * "correcter" place for last_sent_to. --xguo
- */
- asoc->peer.last_sent_to = tp;
+ if (asoc) {
+ asoc->stats.opackets++;
+ if (asoc->peer.last_sent_to != tp)
+ /* Considering the multiple CPU scenario, this is a
+ * "correcter" place for last_sent_to. --xguo
+ */
+ asoc->peer.last_sent_to = tp;
}
if (has_data) {
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index 1b4a7f8..379c81d 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -667,6 +667,7 @@ redo:
chunk->fast_retransmit = SCTP_DONT_FRTX;
q->empty = 0;
+ q->asoc->stats.rtxchunks++;
break;
}
@@ -876,12 +877,14 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)
if (status != SCTP_XMIT_OK) {
/* put the chunk back */
list_add(&chunk->list, &q->control_chunk_list);
- } else if (chunk->chunk_hdr->type == SCTP_CID_FWD_TSN) {
+ } else {
+ asoc->stats.octrlchunks++;
/* PR-SCTP C5) If a FORWARD TSN is sent, the
* sender MUST assure that at least one T3-rtx
* timer is running.
*/
- sctp_transport_reset_timers(transport);
+ if (chunk->chunk_hdr->type == SCTP_CID_FWD_TSN)
+ sctp_transport_reset_timers(transport);
}
break;
@@ -1055,6 +1058,10 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)
*/
if (asoc->state == SCTP_STATE_SHUTDOWN_PENDING)
chunk->chunk_hdr->flags |= SCTP_DATA_SACK_IMM;
+ if (chunk->chunk_hdr->flags & SCTP_DATA_UNORDERED)
+ asoc->stats.ouodchunks++;
+ else
+ asoc->stats.oodchunks++;
break;
@@ -1162,6 +1169,7 @@ int sctp_outq_sack(struct sctp_outq *q, struct sctp_chunk *chunk)
sack_ctsn = ntohl(sack->cum_tsn_ack);
gap_ack_blocks = ntohs(sack->num_gap_ack_blocks);
+ asoc->stats.gapcnt += gap_ack_blocks;
/*
* SFR-CACC algorithm:
* On receipt of a SACK the sender SHOULD execute the
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index e0f01a4..e1c5fc2 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -804,10 +804,11 @@ struct sctp_chunk *sctp_make_sack(const struct sctp_association *asoc)
gabs);
/* Add the duplicate TSN information. */
- if (num_dup_tsns)
+ if (num_dup_tsns) {
+ aptr->stats.idupchunks += num_dup_tsns;
sctp_addto_chunk(retval, sizeof(__u32) * num_dup_tsns,
sctp_tsnmap_get_dups(map));
-
+ }
/* Once we have a sack generated, check to see what our sack
* generation is, if its 0, reset the transports to 0, and reset
* the association generation to 1
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index c076956..c957775 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -542,6 +542,7 @@ static void sctp_do_8_2_transport_strike(sctp_cmd_seq_t *commands,
*/
if (!is_hb || transport->hb_sent) {
transport->rto = min((transport->rto * 2), transport->asoc->rto_max);
+ sctp_max_rto(asoc, transport);
}
}
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index b6adef8..ecf7a17 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -6127,6 +6127,8 @@ static int sctp_eat_data(const struct sctp_association *asoc,
/* The TSN is too high--silently discard the chunk and
* count on it getting retransmitted later.
*/
+ if (chunk->asoc)
+ chunk->asoc->stats.outofseqtsns++;
return SCTP_IERROR_HIGH_TSN;
} else if (tmp > 0) {
/* This is a duplicate. Record it. */
@@ -6226,10 +6228,14 @@ static int sctp_eat_data(const struct sctp_association *asoc,
/* Note: Some chunks may get overcounted (if we drop) or overcounted
* if we renege and the chunk arrives again.
*/
- if (chunk->chunk_hdr->flags & SCTP_DATA_UNORDERED)
+ if (chunk->chunk_hdr->flags & SCTP_DATA_UNORDERED) {
SCTP_INC_STATS(net, SCTP_MIB_INUNORDERCHUNKS);
- else {
+ if (chunk->asoc)
+ chunk->asoc->stats.iuodchunks++;
+ } else {
SCTP_INC_STATS(net, SCTP_MIB_INORDERCHUNKS);
+ if (chunk->asoc)
+ chunk->asoc->stats.iodchunks++;
ordered = 1;
}
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 2e89706..7b8d01a 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -611,6 +611,7 @@ static int sctp_send_asconf_add_ip(struct sock *sk,
2*asoc->pathmtu, 4380));
trans->ssthresh = asoc->peer.i.a_rwnd;
trans->rto = asoc->rto_initial;
+ sctp_max_rto(asoc, trans);
trans->rtt = trans->srtt = trans->rttvar = 0;
sctp_transport_route(trans, NULL,
sctp_sk(asoc->base.sk));
@@ -5635,6 +5636,71 @@ static int sctp_getsockopt_paddr_thresholds(struct sock *sk,
return 0;
}
+/*
+ * SCTP_GET_ASSOC_STATS
+ *
+ * This option retrieves local per endpoint statistics. It is modeled
+ * after OpenSolaris' implementation
+ */
+static int sctp_getsockopt_assoc_stats(struct sock *sk, int len,
+ char __user *optval,
+ int __user *optlen)
+{
+ struct sctp_assoc_stats sas;
+ struct sctp_association *asoc = NULL;
+
+ /* User must provide at least the assoc id */
+ if (len < sizeof(sctp_assoc_t))
+ return -EINVAL;
+
+ if (copy_from_user(&sas, optval, len))
+ return -EFAULT;
+
+ asoc = sctp_id2assoc(sk, sas.sas_assoc_id);
+ if (!asoc)
+ return -EINVAL;
+
+ sas.sas_rtxchunks = asoc->stats.rtxchunks;
+ sas.sas_gapcnt = asoc->stats.gapcnt;
+ sas.sas_outofseqtsns = asoc->stats.outofseqtsns;
+ sas.sas_osacks = asoc->stats.osacks;
+ sas.sas_isacks = asoc->stats.isacks;
+ sas.sas_octrlchunks = asoc->stats.octrlchunks;
+ sas.sas_ictrlchunks = asoc->stats.ictrlchunks;
+ sas.sas_oodchunks = asoc->stats.oodchunks;
+ sas.sas_iodchunks = asoc->stats.iodchunks;
+ sas.sas_ouodchunks = asoc->stats.ouodchunks;
+ sas.sas_iuodchunks = asoc->stats.iuodchunks;
+ sas.sas_idupchunks = asoc->stats.idupchunks;
+ sas.sas_opackets = asoc->stats.opackets;
+ sas.sas_ipackets = asoc->stats.ipackets;
+
+ /* New high max rto observed, will return 0 if not a single
+ * RTO update took place. obs_rto_ipaddr will be bogus
+ * in such a case
+ */
+ sas.sas_maxrto = asoc->stats.max_obs_rto;
+ memcpy(&sas.sas_obs_rto_ipaddr, &asoc->stats.obs_rto_ipaddr,
+ sizeof(struct sockaddr_storage));
+
+ /* Mark beginning of a new observation period */
+ asoc->stats.max_obs_rto = asoc->rto_min;
+
+ /* Allow the struct to grow and fill in as much as possible */
+ len = min_t(size_t, len, sizeof(sas));
+
+ if (put_user(len, optlen))
+ return -EFAULT;
+
+ SCTP_DEBUG_PRINTK("sctp_getsockopt_assoc_stat(%d): %d\n",
+ len, sas.sas_assoc_id);
+
+ if (copy_to_user(optval, &sas, len))
+ return -EFAULT;
+
+ return 0;
+}
+
SCTP_STATIC int sctp_getsockopt(struct sock *sk, int level, int optname,
char __user *optval, int __user *optlen)
{
@@ -5776,6 +5842,9 @@ SCTP_STATIC int sctp_getsockopt(struct sock *sk, int level, int optname,
case SCTP_PEER_ADDR_THLDS:
retval = sctp_getsockopt_paddr_thresholds(sk, optval, len, optlen);
break;
+ case SCTP_GET_ASSOC_STATS:
+ retval = sctp_getsockopt_assoc_stats(sk, len, optval, optlen);
+ break;
default:
retval = -ENOPROTOOPT;
break;
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index 953c21e..40574ad 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -363,6 +363,7 @@ void sctp_transport_update_rto(struct sctp_transport *tp, __u32 rtt)
if (tp->rto > tp->asoc->rto_max)
tp->rto = tp->asoc->rto_max;
+ sctp_max_rto(tp->asoc, tp);
tp->rtt = rtt;
/* Reset rto_pending so that a new RTT measurement is started when a
@@ -620,6 +621,7 @@ void sctp_transport_reset(struct sctp_transport *t)
t->burst_limited = 0;
t->ssthresh = asoc->peer.i.a_rwnd;
t->rto = asoc->rto_initial;
+ sctp_max_rto(asoc, t);
t->rtt = 0;
t->srtt = 0;
t->rttvar = 0;
--
1.8.0.1
^ permalink raw reply related
* Re: Re: Re: [B.A.T.M.A.N.] [PATCH 6/7] batman-adv: Allow to use rntl_link for device creation/deletion
From: Antonio Quartulli @ 2012-12-01 14:54 UTC (permalink / raw)
To: Sven Eckelmann
Cc: The list for a Better Approach To Mobile Ad-hoc Networking,
netdev
In-Reply-To: <3895612.sgriB1OUoY@sven-laptop.home.narfation.org>
[-- Attachment #1: Type: text/plain, Size: 1709 bytes --]
On Sat, Dec 01, 2012 at 02:39:26PM +0100, Sven Eckelmann wrote:
> On Saturday 01 December 2012 14:28:02 Antonio Quartulli wrote:
> > > Because this is the normal way to create virtual network devices (please
> > > feel free to correct me).
> >
> > Well, I've seen different iface types using many tools, e.g. vconfig,
> > tunctl, brctl..
> > Not that this justifies the fact that we should do the same (imho having a
> > standard and unified way for creating interfaces would be the best option).
>
> The device creation and enslaving using vconfig, tunctl and brctl can be
> replaced using ip.
True.
>
> > But, to be honest, I think it should better discuss how to entirely
> > moving/changing the existent API to a "better one" or to a "new one",
> > instead of starting to maintain two of them from now on with no plan, don't
> > you think so?
>
> I leave this discussion to the maintainers of batman-adv.
I started this discussion here because you are still part of them (even if not
listed in MAINTAINERS).
> Btw. removing the
> old one without a time of coexistence sounds like a bad move. And therefore
> maintaining of both interfaces like it is done in other network devices seems
> to be necessary.
>
Exactly, this is what I wanted to discuss as "a plan".
Anyway, I discussed about this together with the others and it seems
that a proper solution now is to wait before merging this patchset and fix the
current sysfs/rtnl_lock problem first. What do you think?
Adding a new API without fixing the current one doesn't sound like a good move.
Cheers,
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v1 3/3] net/mlx4_en: Set number of rx/tx channels using ethtool
From: Or Gerlitz @ 2012-12-01 15:50 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Amir Vadai, David S. Miller, Or Gerlitz, Oren Duer, netdev
In-Reply-To: <1354319925.2640.3.camel@bwh-desktop.uk.solarflarecom.com>
On Sat, Dec 1, 2012 at 1:58 AM, Ben Hutchings <bhutchings@solarflare.com> wrote:
>
> On Thu, 2012-11-29 at 21:21 +0200, Amir Vadai wrote:
> > Add support to changing number of rx/tx channels using
> > ethtool ('ethtool -[lL]'). Where the number of tx channels specified in
> > ethtool
> > is the number of rings per user priority - not total number of tx rings.
> >
> > Signed-off-by: Amir Vadai <amirv@mellanox.com>
> > ---
> > drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 69
> > +++++++++++++++++++++++
> > drivers/net/ethernet/mellanox/mlx4/en_main.c | 2 +-
> > drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 26 +++++----
> > drivers/net/ethernet/mellanox/mlx4/en_tx.c | 2 +-
> > drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 8 ++-
> > 5 files changed, 93 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> > b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> > index dc8ccb4..681bc1b 100644
> > --- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> > +++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> > @@ -999,6 +999,73 @@ static int mlx4_en_set_rxnfc(struct net_device
> > *dev, struct ethtool_rxnfc *cmd)
> [...]
> > +static int mlx4_en_set_channels(struct net_device *dev,
> > + struct ethtool_channels *channel)
> > +{
> > + struct mlx4_en_priv *priv = netdev_priv(dev);
> > + struct mlx4_en_dev *mdev = priv->mdev;
> > + int port_up;
> > + int err = 0;
> > +
> > + if (channel->other_count || channel->combined_count ||
> > + channel->tx_count > channel->max_tx ||
> > + channel->rx_count > channel->max_rx ||
>
> The values of max_tx and max_rx are passed in from userland, so you
> can't trust them.
Is this a general statement re ethtool values passed from user space
or something specific here? can't one assume that the ethtool process
runs under the appropriate admin permission?
>> + !channel->tx_count || !channel->rx_count)
>> + return -EINVAL;
> [...]
^ permalink raw reply
* Re: [PATCH net-next v1 3/3] net/mlx4_en: Set number of rx/tx channels using ethtool
From: Ben Hutchings @ 2012-12-01 16:18 UTC (permalink / raw)
To: Or Gerlitz; +Cc: Amir Vadai, David S. Miller, Or Gerlitz, Oren Duer, netdev
In-Reply-To: <CAJZOPZK98UOsK2HFSC3V6fdzvJfWVKg6pZzxptE97MTgOAk7-w@mail.gmail.com>
On Sat, 2012-12-01 at 17:50 +0200, Or Gerlitz wrote:
> On Sat, Dec 1, 2012 at 1:58 AM, Ben Hutchings <bhutchings@solarflare.com> wrote:
> >
> > On Thu, 2012-11-29 at 21:21 +0200, Amir Vadai wrote:
> > > Add support to changing number of rx/tx channels using
> > > ethtool ('ethtool -[lL]'). Where the number of tx channels specified in
> > > ethtool
> > > is the number of rings per user priority - not total number of tx rings.
> > >
> > > Signed-off-by: Amir Vadai <amirv@mellanox.com>
> > > ---
> > > drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 69
> > > +++++++++++++++++++++++
> > > drivers/net/ethernet/mellanox/mlx4/en_main.c | 2 +-
> > > drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 26 +++++----
> > > drivers/net/ethernet/mellanox/mlx4/en_tx.c | 2 +-
> > > drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 8 ++-
> > > 5 files changed, 93 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> > > b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> > > index dc8ccb4..681bc1b 100644
> > > --- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> > > +++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> > > @@ -999,6 +999,73 @@ static int mlx4_en_set_rxnfc(struct net_device
> > > *dev, struct ethtool_rxnfc *cmd)
> > [...]
> > > +static int mlx4_en_set_channels(struct net_device *dev,
> > > + struct ethtool_channels *channel)
> > > +{
> > > + struct mlx4_en_priv *priv = netdev_priv(dev);
> > > + struct mlx4_en_dev *mdev = priv->mdev;
> > > + int port_up;
> > > + int err = 0;
> > > +
> > > + if (channel->other_count || channel->combined_count ||
> > > + channel->tx_count > channel->max_tx ||
> > > + channel->rx_count > channel->max_rx ||
> >
> > The values of max_tx and max_rx are passed in from userland, so you
> > can't trust them.
>
> Is this a general statement re ethtool values passed from user space
> or something specific here?
General statement.
> can't one assume that the ethtool process
> runs under the appropriate admin permission?
It has CAP_NET_ADMIN, which is not the same thing as CAP_SYS_ADMIN (and
even that doesn't necessarily mean ultimate trust).
Ben.
> >> + !channel->tx_count || !channel->rx_count)
> >> + return -EINVAL;
> > [...]
--
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 net-next v1 3/3] net/mlx4_en: Set number of rx/tx channels using ethtool
From: David Miller @ 2012-12-01 16:25 UTC (permalink / raw)
To: or.gerlitz; +Cc: bhutchings, amirv, ogerlitz, oren, netdev
In-Reply-To: <CAJZOPZK98UOsK2HFSC3V6fdzvJfWVKg6pZzxptE97MTgOAk7-w@mail.gmail.com>
From: Or Gerlitz <or.gerlitz@gmail.com>
Date: Sat, 1 Dec 2012 17:50:09 +0200
> On Sat, Dec 1, 2012 at 1:58 AM, Ben Hutchings <bhutchings@solarflare.com> wrote:
>>
>> The values of max_tx and max_rx are passed in from userland, so you
>> can't trust them.
>
> Is this a general statement re ethtool values passed from user space
> or something specific here? can't one assume that the ethtool process
> runs under the appropriate admin permission?
He's saying that you need to carefully validate the arguments, not
that appropriate permissions haven't been checked.
^ permalink raw reply
* Re: [net-next 0/9][pull request] Intel Wired LAN Driver Updates
From: David Miller @ 2012-12-01 16:29 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, sassmann
In-Reply-To: <1354362817-26168-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Sat, 1 Dec 2012 03:53:28 -0800
> This series contains updates to ixgbe, igb and e1000e. Majority of the
> changes are against igb.
>
> The following are changes since commit 1b4c44e6369dbbafd113f1e00b406f1eda5ab5b2:
> myri10ge: Add vlan rx for better GRO perf.
> and are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master
Pulled, thanks Jeff.
^ permalink raw reply
* Re: Pull request: sfc-next 2012-12-01
From: David Miller @ 2012-12-01 16:33 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1354336821.2640.39.camel@bwh-desktop.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Sat, 1 Dec 2012 04:40:21 +0000
> The following changes since commit bb728820fe7c42fdb838ab2745fb5fe6b18b5ffa:
>
> core: make GRO methods static. (2012-11-29 13:18:32 -0500)
>
> are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next.git for-davem
>
> (commit b9cc977d9d4d1866ee83df38815f4b3b34d99dd9)
>
> 1. More workarounds for TX queue flush failures that can occur during
> interface reconfiguration.
> 2. Fix spurious failure of a firmware request running during a system
> clock change, e.g. ntpd started at the same time as driver load.
> 3. Fix inconsistent statistics after a firmware upgrade.
> 4. Fix a variable (non-)initialisation in offline self-test that can
> make it more disruptive than intended.
> 5. Fix a race that can (at least) cause an assertion failure.
> 6. Miscellaneous cleanup.
Pulled, thanks a lot Ben.
I just want to point out how much I like your commit messages,
your patches are trivial to review because of them.
Thanks again.
^ permalink raw reply
* Re: [PATCH net-next] tcp: change default tcp hash size
From: David Miller @ 2012-12-01 16:36 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1354306132.20109.41.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 30 Nov 2012 12:08:52 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> As time passed, available memory increased faster than number of
> concurrent tcp sockets.
>
> As a result, a machine with 4GB of ram gets a hash table
> with 524288 slots, using 8388608 bytes of memory.
>
> Lets change that by a 16x factor (one slot for 128 KB of ram)
>
> Even if a small machine needs a _lot_ of sockets, tcp lookups are now
> very efficient, using one cache line per socket.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Agreed, applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH v2] ipv6: unify logic evaluating inet6_dev's accept_ra property
From: David Miller @ 2012-12-01 16:37 UTC (permalink / raw)
To: shmulik.ladkani; +Cc: netdev, yoshfuji, tgraf, tore
In-Reply-To: <1354307159-24193-1-git-send-email-shmulik.ladkani@gmail.com>
From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Date: Fri, 30 Nov 2012 22:25:59 +0200
> As of 026359b [ipv6: Send ICMPv6 RSes only when RAs are accepted], the
> logic determining whether to send Router Solicitations is identical
> to the logic determining whether kernel accepts Router Advertisements.
>
> However the condition itself is repeated in several code locations.
>
> Unify it by introducing 'ipv6_accept_ra()' accessor.
>
> Also, simplify the condition expression, making it more readable.
> No semantic change.
>
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] myri10ge: fix incorrect use of ntohs()
From: David Miller @ 2012-12-01 16:37 UTC (permalink / raw)
To: gallatin; +Cc: netdev
In-Reply-To: <1354314686-16149-1-git-send-email-gallatin@myri.com>
From: Andrew Gallatin <gallatin@myri.com>
Date: Fri, 30 Nov 2012 17:31:26 -0500
> 1b4c44e6369dbbafd113f1e00b406f1eda5ab5b2 incorrectly used
> ntohs() rather than htons() in myri10ge_vlan_rx().
>
> Thanks to Fengguang Wu, Yuanhan Liu's kernel-build tester
> for pointing out this bug.
>
> Signed-off-by: Andrew Gallatin <gallatin@myri.com>
Applied.
^ permalink raw reply
* Re: [PATCH -next] qlcnic: remove duplicated include from qlcnic_sysfs.c
From: David Miller @ 2012-12-01 16:37 UTC (permalink / raw)
To: weiyj.lk
Cc: jitendra.kalsaria, sony.chacko, yongjun_wei, linux-driver, netdev
In-Reply-To: <CAPgLHd9rUSTcdop1coRzR00SxwHtetQ5vHwAfLN_Jw1VjaK68g@mail.gmail.com>
From: Wei Yongjun <weiyj.lk@gmail.com>
Date: Sat, 1 Dec 2012 01:01:25 -0500
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> Remove duplicated include.
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Applied.
^ permalink raw reply
* Re: [PATCH 00/17] ATM fixes for pppoatm/br2684
From: David Miller @ 2012-12-01 16:43 UTC (permalink / raw)
To: dwmw2; +Cc: netdev, chas, krzysiek
In-Reply-To: <1354306929.21562.313.camel@shinybook.infradead.org>
From: David Woodhouse <dwmw2@infradead.org>
Date: Fri, 30 Nov 2012 20:22:09 +0000
> Dave, if you're not now ignoring this thread entirely, please pull into
> net-next from
> git://git.infradead.org/users/dwmw2/atm.git
Can you make this actually build first?
drivers/atm/ambassador.c: In function ‘ucode_init’:
drivers/atm/ambassador.c:1972:19: warning: ‘fw’ may be used uninitialized in this function [-Wmaybe-uninitialized]
drivers/atm/solos-pci.c: In function ‘solos_pci_init’:
drivers/atm/solos-pci.c:1329:2: error: size of unnamed array is negative
^ permalink raw reply
* Re: [PATCH 00/17] ATM fixes for pppoatm/br2684
From: David Miller @ 2012-12-01 16:44 UTC (permalink / raw)
To: dwmw2; +Cc: netdev, chas, krzysiek
In-Reply-To: <20121201.114320.103305992136431342.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Sat, 01 Dec 2012 11:43:20 -0500 (EST)
> From: David Woodhouse <dwmw2@infradead.org>
> Date: Fri, 30 Nov 2012 20:22:09 +0000
>
>> Dave, if you're not now ignoring this thread entirely, please pull into
>> net-next from
>> git://git.infradead.org/users/dwmw2/atm.git
>
> Can you make this actually build first?
>
> drivers/atm/ambassador.c: In function ‘ucode_init’:
> drivers/atm/ambassador.c:1972:19: warning: ‘fw’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> drivers/atm/solos-pci.c: In function ‘solos_pci_init’:
> drivers/atm/solos-pci.c:1329:2: error: size of unnamed array is negative
It's from adding the completion to the solos skb cb, you can't do
that. It won't fit on 64-bit when all debugging kconfig options are
enabled.
^ permalink raw reply
* Re: [PATCH 00/17] ATM fixes for pppoatm/br2684
From: David Woodhouse @ 2012-12-01 16:48 UTC (permalink / raw)
To: David Miller; +Cc: netdev, chas, krzysiek
In-Reply-To: <20121201.114440.331740099591161757.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 404 bytes --]
On Sat, 2012-12-01 at 11:44 -0500, David Miller wrote:
> It's from adding the completion to the solos skb cb, you can't do
> that. It won't fit on 64-bit when all debugging kconfig options are
> enabled.
Aha, thanks. Will sort that out. Apologies.
The ambassador one may be related to a separate patch which fixes the
endless loop in firmware loading? Not in my tree, though.
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]
^ permalink raw reply
* Re: [PATCH 00/17] ATM fixes for pppoatm/br2684
From: Chas Williams (CONTRACTOR) @ 2012-12-01 17:02 UTC (permalink / raw)
To: David Woodhouse; +Cc: David Miller, netdev, krzysiek
In-Reply-To: <1354380532.21562.345.camel@shinybook.infradead.org>
In message <1354380532.21562.345.camel@shinybook.infradead.org>,David Woodhouse writes:
>On Sat, 2012-12-01 at 11:44 -0500, David Miller wrote:
>> It's from adding the completion to the solos skb cb, you can't do
>> that. It won't fit on 64-bit when all debugging kconfig options are
>> enabled.
>
>Aha, thanks. Will sort that out. Apologies.
>
>The ambassador one may be related to a separate patch which fixes the
>endless loop in firmware loading? Not in my tree, though.
i think the warning with the ambassador dates back to before the most
recent patch. probably to the conversion to use the firmware loader.
^ permalink raw reply
* Re: [PATCH 00/17] ATM fixes for pppoatm/br2684
From: David Woodhouse @ 2012-12-01 17:21 UTC (permalink / raw)
To: Chas Williams (CONTRACTOR); +Cc: David Miller, netdev, krzysiek
In-Reply-To: <201212011702.qB1H2v07010546@thirdoffive.cmf.nrl.navy.mil>
[-- Attachment #1: Type: text/plain, Size: 984 bytes --]
On Sat, 2012-12-01 at 12:02 -0500, Chas Williams (CONTRACTOR) wrote:
> In message <1354380532.21562.345.camel@shinybook.infradead.org>,David Woodhouse writes:
> >On Sat, 2012-12-01 at 11:44 -0500, David Miller wrote:
> >> It's from adding the completion to the solos skb cb, you can't do
> >> that. It won't fit on 64-bit when all debugging kconfig options are
> >> enabled.
> >
> >Aha, thanks. Will sort that out. Apologies.
> >
> >The ambassador one may be related to a separate patch which fixes the
> >endless loop in firmware loading? Not in my tree, though.
>
> i think the warning with the ambassador dates back to before the most
> recent patch. probably to the conversion to use the firmware loader.
Possibly not even a bug at all, in fact — GCC is fairly loose with those
warnings. But if it is a bug it's my fault. I'll take a look at that
too. Not tonight though; I'm going out shortly and will only just manage
the solos-pci fix.
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]
^ permalink raw reply
* Re: sky2: Correct mistakenly switched read/write sequence
From: David Miller @ 2012-12-01 17:27 UTC (permalink / raw)
To: LinoSanfilippo; +Cc: shemminger, mlindner, netdev
In-Reply-To: <20121201124239.GB3914@neptun>
From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Date: Sat, 1 Dec 2012 13:42:39 +0100
> In sky2_all_down() the order of the read()/write() access to B0_IMSK seems to
> be mistakenly switched. The original intention was obviously to avoid PCI write
> posting.
> This patch fixes the order.
>
> Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
I would say that no such intention exists at all.
The read is there because a long time ago the result as used
to compute an 'imask' variable.
Please see commit:
commit d72ff8fa7f8b344382963721f842256825c4660b
Author: Mike McCormack <mikem@ring3k.org>
Date: Thu May 13 06:12:51 2010 +0000
sky2: Refactor down/up code out of sky2_restart()
Code to bring down all sky2 interfaces and bring it up
again can be reused in sky2_suspend and sky2_resume.
Factor the code to bring the interfaces down into
sky2_all_down and the up code into sky2_all_up.
Signed-off-by: Mike McCormack <mikem@ring3k.org>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* [PATCH] net: fix possible deadlocks in rtnl_trylock/unlock
From: Simon Wunderlich @ 2012-12-01 17:29 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r, Simon Wunderlich
If rtnl_trylock() is used to prevent circular dependencies, rtnl_unlock()
may destroy this attempt because it first unlocks rtnl_mutex but may
lock it again later. The callgraph roughly looks like:
rtnl_unlock()
netdev_run_todo()
__rtnl_unlock()
netdev_wait_allrefs()
rtnl_lock()
...
__rtnl_unlock()
There are two users which have possible deadlocks and are fixed in this
patch: batman-adv and bridge. Their problematic access pattern is the same:
notifier_call handler:
* holds rtnl lock when called
* calls sysfs code at some point (acquiring sysfs locks)
sysfs code:
* holds sysfs lock when called
* calls rtnl_trylock() and rtnl_unlock(), rtnl_unlock() calls rtnl_lock
implicitly
Fix this by exporting __rtnl_unlock() to just unlock the mutex without
implicitly locking again.
Reported-by: Sven Eckelmann <sven-KaDOiPu9UxWEi8DpZVb4nw@public.gmane.org>
Signed-off-by: Simon Wunderlich <siwu-MaAgPAbsBIVS8oHt8HbXEIQuADTiUCJX@public.gmane.org>
---
Of course, an alternative would be to not lock again after unlocking
within rtnl_unlock(), or put the todo handling into the locked section.
I'm not familiar enough with this code to know what would be best.
There are others using rtnl_trylock(), but I'm not sure if they
are affected.
---
net/batman-adv/sysfs.c | 2 +-
net/bridge/br_sysfs_br.c | 2 +-
net/bridge/br_sysfs_if.c | 2 +-
net/core/rtnetlink.c | 1 +
4 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
index 66518c7..41b74aa 100644
--- a/net/batman-adv/sysfs.c
+++ b/net/batman-adv/sysfs.c
@@ -635,7 +635,7 @@ static ssize_t batadv_store_mesh_iface(struct kobject *kobj,
ret = batadv_hardif_enable_interface(hard_iface, buff);
unlock:
- rtnl_unlock();
+ __rtnl_unlock();
out:
batadv_hardif_free_ref(hard_iface);
return ret;
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index c5c0593..c122782 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -142,7 +142,7 @@ static ssize_t store_stp_state(struct device *d,
if (!rtnl_trylock())
return restart_syscall();
br_stp_set_enabled(br, val);
- rtnl_unlock();
+ __rtnl_unlock();
return len;
}
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index 13b36bd..d99f394 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -223,7 +223,7 @@ static ssize_t brport_store(struct kobject * kobj,
if (ret == 0)
ret = count;
}
- rtnl_unlock();
+ __rtnl_unlock();
}
return ret;
}
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index fad649a..d95ba6f 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -72,6 +72,7 @@ void __rtnl_unlock(void)
{
mutex_unlock(&rtnl_mutex);
}
+EXPORT_SYMBOL(__rtnl_unlock);
void rtnl_unlock(void)
{
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH 00/17] ATM fixes for pppoatm/br2684
From: David Woodhouse @ 2012-12-01 17:33 UTC (permalink / raw)
To: David Miller; +Cc: netdev, chas, krzysiek
In-Reply-To: <20121201.114440.331740099591161757.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 2136 bytes --]
On Sat, 2012-12-01 at 11:44 -0500, David Miller wrote:
>
> > drivers/atm/solos-pci.c: In function ‘solos_pci_init’:
> > drivers/atm/solos-pci.c:1329:2: error: size of unnamed array is
> negative
>
> It's from adding the completion to the solos skb cb, you can't do
> that. It won't fit on 64-bit when all debugging kconfig options are
> enabled.
This is a sane use of skb refcounting, right? It seems to work, so if
it's OK I'll redo the offending patch with this change to avoid breaking
the bisection, and submit a new pull request.
Very glad I added the BUILD_BUG_ON on the cb struct size now. Perhaps
there should be a generic helper for that? Something like
skb_cb_cast(struct foo_cb, skb) could do it automatically...?
diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index e59bcfd..6fe62c5 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -92,7 +92,6 @@ struct pkt_hdr {
};
struct solos_skb_cb {
- struct completion c;
struct atm_vcc *vcc;
uint32_t dma_addr;
};
@@ -853,14 +852,15 @@ static void pclose(struct atm_vcc *vcc)
header->vci = cpu_to_le16(vcc->vci);
header->type = cpu_to_le16(PKT_PCLOSE);
- init_completion(&SKB_CB(skb)->c);
-
+ skb_get(skb);
fpga_queue(card, port, skb, NULL);
- if (!wait_for_completion_timeout(&SKB_CB(skb)->c, 5 * HZ))
+ if (!wait_event_timeout(card->param_wq, !skb_shared(skb), 5 * HZ))
dev_warn(&card->dev->dev, "Timeout waiting for VCC close on port %d\n",
port);
+ dev_kfree_skb_any(skb);
+
/* Hold up vcc_destroy_socket() (our caller) until solos_bh() in the
tasklet has finished processing any incoming packets (and, more to
the point, using the vcc pointer). */
@@ -990,10 +990,8 @@ static uint32_t fpga_tx(struct solos_card *card)
atomic_inc(&vcc->stats->tx);
solos_pop(vcc, oldskb);
} else {
- struct pkt_hdr *header = (void *)oldskb->data;
- if (le16_to_cpu(header->type) == PKT_PCLOSE)
- complete(&SKB_CB(oldskb)->c);
dev_kfree_skb_irq(oldskb);
+ wake_up(&card->param_wq);
}
}
}
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]
^ permalink raw reply related
* Re: [B.A.T.M.A.N.] [PATCH] net: fix possible deadlocks in rtnl_trylock/unlock
From: Sven Eckelmann @ 2012-12-01 18:07 UTC (permalink / raw)
To: b.a.t.m.a.n, bridge, linux-rdma, linux-s390, Andy Gospodarek,
Jay Vosburgh, Divy Le Ray
Cc: Simon Wunderlich, davem, netdev, Simon Wunderlich
In-Reply-To: <1354382991-31350-1-git-send-email-siwu@hrz.tu-chemnitz.de>
[-- Attachment #1: Type: text/plain, Size: 3266 bytes --]
On Saturday 01 December 2012 18:29:51 Simon Wunderlich wrote:
> If rtnl_trylock() is used to prevent circular dependencies, rtnl_unlock()
> may destroy this attempt because it first unlocks rtnl_mutex but may
> lock it again later. The callgraph roughly looks like:
>
> rtnl_unlock()
> netdev_run_todo()
> __rtnl_unlock()
> netdev_wait_allrefs()
> rtnl_lock()
> ...
> __rtnl_unlock()
>
> There are two users which have possible deadlocks and are fixed in this
> patch: batman-adv and bridge. Their problematic access pattern is the same:
>
> notifier_call handler:
> * holds rtnl lock when called
> * calls sysfs code at some point (acquiring sysfs locks)
>
> sysfs code:
> * holds sysfs lock when called
> * calls rtnl_trylock() and rtnl_unlock(), rtnl_unlock() calls rtnl_lock
> implicitly
>
> Fix this by exporting __rtnl_unlock() to just unlock the mutex without
> implicitly locking again.
>
> Reported-by: Sven Eckelmann <sven@narfation.org>
> Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
>
> ---
> Of course, an alternative would be to not lock again after unlocking
> within rtnl_unlock(), or put the todo handling into the locked section.
> I'm not familiar enough with this code to know what would be best.
>
> There are others using rtnl_trylock(), but I'm not sure if they
> are affected.
At least they look like they have a problem in a parallel user scenario
involving another lock and locking order (most of them s_active or a device
lock). So just to list the places and poke some other users. They can better
decide for themself because they know the code.
drivers/infiniband/ulp/ipoib/ipoib_cm.c: if (!rtnl_trylock())
drivers/infiniband/ulp/ipoib/ipoib_vlan.c: if (!rtnl_trylock())
drivers/infiniband/ulp/ipoib/ipoib_vlan.c: if (!rtnl_trylock())
drivers/net/bonding/bond_alb.c: if (!rtnl_trylock()) {
drivers/net/bonding/bond_main.c: if (!rtnl_trylock()) {
drivers/net/bonding/bond_main.c: if (!rtnl_trylock()) {
drivers/net/bonding/bond_main.c: if (!rtnl_trylock()) {
drivers/net/bonding/bond_main.c: if (!rtnl_trylock()) {
drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock())
drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock())
drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock())
drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock())
drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock())
drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock())
drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c: if (!rtnl_trylock()) /*
synchronize with ifdown */
drivers/s390/net/qeth_l2_main.c: if (rtnl_trylock()) {
drivers/s390/net/qeth_l3_main.c: if (rtnl_trylock()) {
net/bridge/br_sysfs_br.c: if (!rtnl_trylock())
net/bridge/br_sysfs_if.c: if (!rtnl_trylock())
net/core/net-sysfs.c: if (!rtnl_trylock())
net/core/net-sysfs.c: if (!rtnl_trylock())
net/core/net-sysfs.c: if (!rtnl_trylock())
net/core/net-sysfs.c: if (!rtnl_trylock())
net/core/net-sysfs.c: if (!rtnl_trylock())
net/ipv4/devinet.c: if (!rtnl_trylock()) {
net/ipv6/addrconf.c: if (!rtnl_trylock())
net/ipv6/addrconf.c: if (!rtnl_trylock())
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [net-next:master 100/106] net/ipv4/inet_hashtables.c:242:21: sparse: restricted __portpair degrades to integer
From: kbuild test robot @ 2012-12-01 18:20 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
head: 577b981714b0b3530817569bf705bd74881efc83
commit: ce43b03e8889475817d427b1f3724c7e294b76eb [100/106] net: move inet_dport/inet_num in sock_common
sparse warnings:
+ net/ipv4/inet_hashtables.c:242:21: sparse: restricted __portpair degrades to integer
net/ipv4/inet_hashtables.c:246:29: sparse: restricted __portpair degrades to integer
net/ipv4/inet_hashtables.c:267:21: sparse: restricted __portpair degrades to integer
net/ipv4/inet_hashtables.c:274:29: sparse: restricted __portpair degrades to integer
net/ipv4/inet_hashtables.c:326:21: sparse: restricted __portpair degrades to integer
net/ipv4/inet_hashtables.c:341:21: sparse: restricted __portpair degrades to integer
--
+ net/ipv6/inet6_hashtables.c:92:21: sparse: restricted __portpair degrades to integer
net/ipv6/inet6_hashtables.c:95:29: sparse: restricted __portpair degrades to integer
net/ipv6/inet6_hashtables.c:111:21: sparse: restricted __portpair degrades to integer
net/ipv6/inet6_hashtables.c:117:29: sparse: restricted __portpair degrades to integer
net/ipv6/inet6_hashtables.c:248:21: sparse: restricted __portpair degrades to integer
net/ipv6/inet6_hashtables.c:263:21: sparse: restricted __portpair degrades to integer
vim +242 net/ipv4/inet_hashtables.c
77a5ba55 Pavel Emelyanov 2007-12-20 226 INET_ADDR_COOKIE(acookie, saddr, daddr)
77a5ba55 Pavel Emelyanov 2007-12-20 227 const __portpair ports = INET_COMBINED_PORTS(sport, hnum);
77a5ba55 Pavel Emelyanov 2007-12-20 228 struct sock *sk;
3ab5aee7 Eric Dumazet 2008-11-16 229 const struct hlist_nulls_node *node;
77a5ba55 Pavel Emelyanov 2007-12-20 230 /* Optimize here for direct hit, only listening connections can
77a5ba55 Pavel Emelyanov 2007-12-20 231 * have wildcards anyways.
77a5ba55 Pavel Emelyanov 2007-12-20 232 */
9f26b3ad Pavel Emelyanov 2008-06-16 233 unsigned int hash = inet_ehashfn(net, daddr, hnum, saddr, sport);
f373b53b Eric Dumazet 2009-10-09 234 unsigned int slot = hash & hashinfo->ehash_mask;
3ab5aee7 Eric Dumazet 2008-11-16 235 struct inet_ehash_bucket *head = &hashinfo->ehash[slot];
77a5ba55 Pavel Emelyanov 2007-12-20 236
3ab5aee7 Eric Dumazet 2008-11-16 237 rcu_read_lock();
3ab5aee7 Eric Dumazet 2008-11-16 238 begin:
3ab5aee7 Eric Dumazet 2008-11-16 239 sk_nulls_for_each_rcu(sk, node, &head->chain) {
ce43b03e Eric Dumazet 2012-11-30 240 if (sk->sk_hash != hash)
ce43b03e Eric Dumazet 2012-11-30 241 continue;
ce43b03e Eric Dumazet 2012-11-30 @242 if (likely(INET_MATCH(sk, net, acookie,
ce43b03e Eric Dumazet 2012-11-30 243 saddr, daddr, ports, dif))) {
3ab5aee7 Eric Dumazet 2008-11-16 244 if (unlikely(!atomic_inc_not_zero(&sk->sk_refcnt)))
3ab5aee7 Eric Dumazet 2008-11-16 245 goto begintw;
ce43b03e Eric Dumazet 2012-11-30 246 if (unlikely(!INET_MATCH(sk, net, acookie,
ce43b03e Eric Dumazet 2012-11-30 247 saddr, daddr, ports, dif))) {
3ab5aee7 Eric Dumazet 2008-11-16 248 sock_put(sk);
3ab5aee7 Eric Dumazet 2008-11-16 249 goto begin;
3ab5aee7 Eric Dumazet 2008-11-16 250 }
---
0-DAY kernel build testing backend Open Source Technology Center
Fengguang Wu, Yuanhan Liu Intel Corporation
^ permalink raw reply
* Re: [PATCH] net: fix possible deadlocks in rtnl_trylock/unlock
From: Eric Dumazet @ 2012-12-01 18:36 UTC (permalink / raw)
To: Simon Wunderlich; +Cc: davem, netdev, b.a.t.m.a.n, Simon Wunderlich
In-Reply-To: <1354382991-31350-1-git-send-email-siwu@hrz.tu-chemnitz.de>
On Sat, 2012-12-01 at 18:29 +0100, Simon Wunderlich wrote:
> If rtnl_trylock() is used to prevent circular dependencies, rtnl_unlock()
> may destroy this attempt because it first unlocks rtnl_mutex but may
> lock it again later. The callgraph roughly looks like:
>
> rtnl_unlock()
> netdev_run_todo()
> __rtnl_unlock()
> netdev_wait_allrefs()
> rtnl_lock()
> ...
> __rtnl_unlock()
>
> There are two users which have possible deadlocks and are fixed in this
> patch: batman-adv and bridge. Their problematic access pattern is the same:
>
> notifier_call handler:
> * holds rtnl lock when called
> * calls sysfs code at some point (acquiring sysfs locks)
>
> sysfs code:
> * holds sysfs lock when called
> * calls rtnl_trylock() and rtnl_unlock(), rtnl_unlock() calls rtnl_lock
> implicitly
>
> Fix this by exporting __rtnl_unlock() to just unlock the mutex without
> implicitly locking again.
>
> Reported-by: Sven Eckelmann <sven@narfation.org>
> Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
>
> ---
> Of course, an alternative would be to not lock again after unlocking
> within rtnl_unlock(), or put the todo handling into the locked section.
> I'm not familiar enough with this code to know what would be best.
>
> There are others using rtnl_trylock(), but I'm not sure if they
> are affected.
> ---
> net/batman-adv/sysfs.c | 2 +-
> net/bridge/br_sysfs_br.c | 2 +-
> net/bridge/br_sysfs_if.c | 2 +-
> net/core/rtnetlink.c | 1 +
> 4 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
> index 66518c7..41b74aa 100644
> --- a/net/batman-adv/sysfs.c
> +++ b/net/batman-adv/sysfs.c
> @@ -635,7 +635,7 @@ static ssize_t batadv_store_mesh_iface(struct kobject *kobj,
> ret = batadv_hardif_enable_interface(hard_iface, buff);
>
> unlock:
> - rtnl_unlock();
> + __rtnl_unlock();
> out:
> batadv_hardif_free_ref(hard_iface);
> return ret;
> diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
> index c5c0593..c122782 100644
> --- a/net/bridge/br_sysfs_br.c
> +++ b/net/bridge/br_sysfs_br.c
> @@ -142,7 +142,7 @@ static ssize_t store_stp_state(struct device *d,
> if (!rtnl_trylock())
> return restart_syscall();
> br_stp_set_enabled(br, val);
> - rtnl_unlock();
> + __rtnl_unlock();
>
> return len;
> }
I have no idea of why you believe there is a problem here.
Could you explain how net_todo_list could be not empty ?
As long as no device is unregistered between
rtnl_trylock()/rtnl_unlock(), there is no possible deadlock.
^ permalink raw reply
* Re: sky2: Correct mistakenly switched read/write sequence
From: Stephen Hemminger @ 2012-12-01 18:36 UTC (permalink / raw)
To: Lino Sanfilippo; +Cc: mlindner, davem, netdev
In-Reply-To: <20121201124239.GB3914@neptun>
On Sat, 1 Dec 2012 13:42:39 +0100
Lino Sanfilippo <LinoSanfilippo@gmx.de> wrote:
> In sky2_all_down() the order of the read()/write() access to B0_IMSK seems to
> be mistakenly switched. The original intention was obviously to avoid PCI write
> posting.
> This patch fixes the order.
>
> Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
You are both right. David is correct in that the original code here
was quite different and was doing save/restore of irq mask.
That changed as driver evolved to only bring up IRQ if device was
brought up. At which point the read of irq mask was a left over call.
Lino is correct, it makes sense to do read after write to ensure PCI posting.
So I agree with the patch, but would like the commit message changed
slightly.
^ permalink raw reply
* [PATCH net-next] sundance: Enable WoL support
From: Denis Kirjanov @ 2012-12-01 18:39 UTC (permalink / raw)
To: netdev; +Cc: davem, kda
In-Reply-To: <Denis Kirjanov <kda@linux-powerpc.org>
Enable WoL support.
Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
drivers/net/ethernet/dlink/sundance.c | 81 ++++++++++++++++++++++++++++++++-
1 files changed, 80 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/dlink/sundance.c b/drivers/net/ethernet/dlink/sundance.c
index 3b83588..bbeb4c7 100644
--- a/drivers/net/ethernet/dlink/sundance.c
+++ b/drivers/net/ethernet/dlink/sundance.c
@@ -259,6 +259,7 @@ enum alta_offsets {
EECtrl = 0x36,
FlashAddr = 0x40,
FlashData = 0x44,
+ WakeEvent = 0x45,
TxStatus = 0x46,
TxFrameId = 0x47,
DownCounter = 0x18,
@@ -333,6 +334,14 @@ enum mac_ctrl1_bits {
RxEnable=0x0800, RxDisable=0x1000, RxEnabled=0x2000,
};
+/* Bits in WakeEvent register. */
+enum wake_event_bits {
+ WakePktEnable = 0x01,
+ MagicPktEnable = 0x02,
+ LinkEventEnable = 0x04,
+ WolEnable = 0x80,
+};
+
/* The Rx and Tx buffer descriptors. */
/* Note that using only 32 bit fields simplifies conversion to big-endian
architectures. */
@@ -392,6 +401,7 @@ struct netdev_private {
unsigned int default_port:4; /* Last dev->if_port value. */
unsigned int an_enable:1;
unsigned int speed;
+ unsigned int wol_enabled:1; /* Wake on LAN enabled */
struct tasklet_struct rx_tasklet;
struct tasklet_struct tx_tasklet;
int budget;
@@ -829,7 +839,7 @@ static int netdev_open(struct net_device *dev)
unsigned long flags;
int i;
- /* Do we need to reset the chip??? */
+ sundance_reset(dev, 0x00ff << 16);
i = request_irq(irq, intr_handler, IRQF_SHARED, dev->name, dev);
if (i)
@@ -877,6 +887,10 @@ static int netdev_open(struct net_device *dev)
iowrite16 (StatsEnable | RxEnable | TxEnable, ioaddr + MACCtrl1);
+ /* Disable Wol */
+ iowrite8(ioread8(ioaddr + WakeEvent) | 0x00, ioaddr + WakeEvent);
+ np->wol_enabled = 0;
+
if (netif_msg_ifup(np))
printk(KERN_DEBUG "%s: Done netdev_open(), status: Rx %x Tx %x "
"MAC Control %x, %4.4x %4.4x.\n",
@@ -1715,6 +1729,60 @@ static void get_ethtool_stats(struct net_device *dev,
data[i++] = np->xstats.rx_mcasts;
}
+#ifdef CONFIG_PM
+
+static void sundance_get_wol(struct net_device *dev,
+ struct ethtool_wolinfo *wol)
+{
+ struct netdev_private *np = netdev_priv(dev);
+ void __iomem *ioaddr = np->base;
+ u8 wol_bits;
+
+ wol->wolopts = 0;
+
+ wol->supported = (WAKE_PHY | WAKE_MAGIC);
+ if (!np->wol_enabled)
+ return;
+
+ wol_bits = ioread8(ioaddr + WakeEvent);
+ if (wol_bits & MagicPktEnable)
+ wol->wolopts |= WAKE_MAGIC;
+ if (wol_bits & LinkEventEnable)
+ wol->wolopts |= WAKE_PHY;
+}
+
+static int sundance_set_wol(struct net_device *dev,
+ struct ethtool_wolinfo *wol)
+{
+ struct netdev_private *np = netdev_priv(dev);
+ void __iomem *ioaddr = np->base;
+ u8 wol_bits;
+
+ if (!device_can_wakeup(&np->pci_dev->dev))
+ return -EOPNOTSUPP;
+
+ np->wol_enabled = !!(wol->wolopts);
+ wol_bits = ioread8(ioaddr + WakeEvent);
+ wol_bits &= ~(WakePktEnable | MagicPktEnable |
+ LinkEventEnable | WolEnable);
+
+ if (np->wol_enabled) {
+ if (wol->wolopts & WAKE_MAGIC)
+ wol_bits |= (MagicPktEnable | WolEnable);
+ if (wol->wolopts & WAKE_PHY)
+ wol_bits |= (LinkEventEnable | WolEnable);
+ }
+ iowrite8(wol_bits, ioaddr + WakeEvent);
+
+ device_set_wakeup_enable(&np->pci_dev->dev, np->wol_enabled);
+
+ return 0;
+}
+#else
+#define sundance_get_wol NULL
+#define sundance_set_wol NULL
+#endif /* CONFIG_PM */
+
static const struct ethtool_ops ethtool_ops = {
.begin = check_if_running,
.get_drvinfo = get_drvinfo,
@@ -1722,6 +1790,8 @@ static const struct ethtool_ops ethtool_ops = {
.set_settings = set_settings,
.nway_reset = nway_reset,
.get_link = get_link,
+ .get_wol = sundance_get_wol,
+ .set_wol = sundance_set_wol,
.get_msglevel = get_msglevel,
.set_msglevel = set_msglevel,
.get_strings = get_strings,
@@ -1867,6 +1937,8 @@ static void __devexit sundance_remove1 (struct pci_dev *pdev)
static int sundance_suspend(struct pci_dev *pci_dev, pm_message_t state)
{
struct net_device *dev = pci_get_drvdata(pci_dev);
+ struct netdev_private *np = netdev_priv(dev);
+ void __iomem *ioaddr = np->base;
if (!netif_running(dev))
return 0;
@@ -1875,6 +1947,12 @@ static int sundance_suspend(struct pci_dev *pci_dev, pm_message_t state)
netif_device_detach(dev);
pci_save_state(pci_dev);
+ if (np->wol_enabled) {
+ iowrite8(AcceptBroadcast | AcceptMyPhys, ioaddr + RxMode);
+ iowrite16(RxEnable, ioaddr + MACCtrl1);
+ }
+ pci_enable_wake(pci_dev, pci_choose_state(pci_dev, state),
+ np->wol_enabled);
pci_set_power_state(pci_dev, pci_choose_state(pci_dev, state));
return 0;
@@ -1890,6 +1968,7 @@ static int sundance_resume(struct pci_dev *pci_dev)
pci_set_power_state(pci_dev, PCI_D0);
pci_restore_state(pci_dev);
+ pci_enable_wake(pci_dev, PCI_D0, 0);
err = netdev_open(dev);
if (err) {
--
1.7.3.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox