* Re: [net-next PATCH] net: codel: Avoid undefined behavior from signed overflow
From: Eric Dumazet @ 2013-10-31 15:10 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Jesper Dangaard Brouer, netdev, Paul E. McKenney, Dave Taht
In-Reply-To: <20131031151551.675ab908@redhat.com>
On Thu, 2013-10-31 at 15:15 +0100, Jesper Dangaard Brouer wrote:
> Okay, I'll cook up another patch, after work.
>
> Adding all the typecheck() stuff, just bloats the code.
>
> Would it be better/okay just to do?:
> (s32)((u32)(a) - (u32)(b)) > 0)
>
>
What about using the existing codel types ?
diff --git a/include/net/codel.h b/include/net/codel.h
index 389cf62..89a7781 100644
--- a/include/net/codel.h
+++ b/include/net/codel.h
@@ -72,7 +72,12 @@ static inline codel_time_t codel_get_time(void)
return ns >> CODEL_SHIFT;
}
-#define codel_time_after(a, b) ((s32)(a) - (s32)(b) > 0)
+static inline bool codel_time_after(codel_time_t a, codel_time_t b)
+{
+ codel_tdiff_t delta = a - b;
+
+ return delta >= 0;
+}
#define codel_time_after_eq(a, b) ((s32)(a) - (s32)(b) >= 0)
#define codel_time_before(a, b) ((s32)(a) - (s32)(b) < 0)
#define codel_time_before_eq(a, b) ((s32)(a) - (s32)(b) <= 0)
You need of course something similar for all variants.
^ permalink raw reply related
* [PATCH net-next] tcp: enable sockets to use MSG_FASTOPEN by default
From: Yuchung Cheng @ 2013-10-31 16:19 UTC (permalink / raw)
To: davem, edumazet; +Cc: netdev, ncardwell, sivasankar, Yuchung Cheng
Applications have started to use Fast Open (e.g., Chrome browser has
such an optional flag) and the feature has gone through several
generations of kernels since 3.7 with many real network tests. It's
time to enable this flag by default for applications to test more
conveniently and extensively.
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
Documentation/networking/ip-sysctl.txt | 4 ++--
net/ipv4/tcp_fastopen.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index a46d785..6c00983 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -451,7 +451,7 @@ tcp_fastopen - INTEGER
connect() to perform a TCP handshake automatically.
The values (bitmap) are
- 1: Enables sending data in the opening SYN on the client.
+ 1: Enables sending data in the opening SYN on the client w/ MSG_FASTOPEN.
2: Enables TCP Fast Open on the server side, i.e., allowing data in
a SYN packet to be accepted and passed to the application before
3-way hand shake finishes.
@@ -464,7 +464,7 @@ tcp_fastopen - INTEGER
different ways of setting max_qlen without the TCP_FASTOPEN socket
option.
- Default: 0
+ Default: 1
Note that the client & server side Fast Open flags (1 and 2
respectively) must be also enabled before the rest of flags can take
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 766032b..f195d93 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -8,7 +8,7 @@
#include <net/inetpeer.h>
#include <net/tcp.h>
-int sysctl_tcp_fastopen __read_mostly;
+int sysctl_tcp_fastopen __read_mostly = TFO_CLIENT_ENABLE;
struct tcp_fastopen_context __rcu *tcp_fastopen_ctx;
--
1.8.4.1
^ permalink raw reply related
* Re: [PATCH v2 net-next] net: pkt_sched: PIE AQM scheme
From: Eric Dumazet @ 2013-10-31 16:52 UTC (permalink / raw)
To: Vijay Subramanian; +Cc: netdev, davem, shemminger, Mythili Prabhu, Dave Taht
In-Reply-To: <1383167922-22495-1-git-send-email-subramanian.vijay@gmail.com>
On Wed, 2013-10-30 at 14:18 -0700, Vijay Subramanian wrote:
> +static void pie_destroy(struct Qdisc *sch)
> +{
> + struct pie_sched_data *q = qdisc_priv(sch);
> + del_timer_sync(&q->adapt_timer);
> +}
This doesn't look safe to me.
timer can rearm itself.
Take a look at commit 980c478ddbb720948967b028ddbb4179a025bc2c
^ permalink raw reply
* Re: [PATCH v2 net-next] net: pkt_sched: PIE AQM scheme
From: Eric Dumazet @ 2013-10-31 17:03 UTC (permalink / raw)
To: Vijay Subramanian; +Cc: netdev, davem, shemminger, Mythili Prabhu, Dave Taht
In-Reply-To: <1383167922-22495-1-git-send-email-subramanian.vijay@gmail.com>
On Wed, 2013-10-30 at 14:18 -0700, Vijay Subramanian wrote:
+ if (tb[TCA_PIE_TUPDATE]) {
+ /* tupdate is in us */
+ u32 tupdate = nla_get_u32(tb[TCA_PIE_TUPDATE]);
+ /* convert to pschedtime */
+ q->params.tupdate = PSCHED_NS2TICKS((u64) tupdate * NSEC_PER_USEC);
+ }
...
> +static void pie_timer(unsigned long arg)
> +{
> + struct Qdisc *sch = (struct Qdisc *)arg;
> + struct pie_sched_data *q = qdisc_priv(sch);
> + u32 tup;
> + spinlock_t *root_lock = qdisc_lock(qdisc_root_sleeping(sch));
> +
> + spin_lock(root_lock);
> + calculate_probability(sch);
> +
> + /* reset the timer to fire after 'tupdate'. tupdate is currently in
> + * psched_time; mod_timer expects time to be in jiffies so convert from
> + * pschedtime to jiffies
> + */
> + tup = PSCHED_TICKS2NS(q->params.tupdate);
> + tup = tup / NSEC_PER_MSEC;
> + tup = (tup * HZ) / MSEC_PER_SEC;
> +
> + mod_timer(&q->adapt_timer, jiffies + tup);
> + spin_unlock(root_lock);
> +
> +}
Oh well, this makes no sense.
mod_timer() resolution is in jiffies, so please maintains params.tupdate
in usec, and use usecs_to_jiffies()
mod_timer(&q->adapt_timer, jiffies +
usecs_to_jiffies(q->params.tupdate));
^ permalink raw reply
* [PATCH net-next] tcp: properly handle stretch acks in slow start
From: Yuchung Cheng @ 2013-10-31 18:07 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, ncardwell, vanj, Yuchung Cheng
Slow start now increases cwnd by 1 if an ACK acknowledges some packets,
regardless the number of packets. Consequently slow start performance
is highly dependent on the degree of the stretch ACKs caused by
receiver or network ACK compression mechanisms (e.g., delayed-ACK,
GRO, etc). But slow start algorithm is to send twice the amount of
packets of packets left so it should process a stretch ACK of degree
N as if N ACKs of degree 1, then exits when cwnd exceeds ssthresh. A
follow up patch will use the remainder of the N (if greater than 1)
to adjust cwnd in the congestion avoidance phase.
In addition this patch retires the experimental limited slow start
(LSS) feature. LSS has multiple drawbacks but questionable benefit. The
fractional cwnd increase in LSS requires a loop in slow start even
though it's rarely used. Configuring such an increase step via a global
sysctl on different BDPS seems hard. Finally and most importantly the
slow start overshoot concern is now better covered by the Hybrid slow
start (hystart) enabled by default.
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
Documentation/networking/ip-sysctl.txt | 11 --------
include/net/tcp.h | 7 +++--
net/ipv4/sysctl_net_ipv4.c | 7 -----
net/ipv4/tcp_bic.c | 5 ++--
net/ipv4/tcp_cong.c | 47 ++++++++++++----------------------
net/ipv4/tcp_cubic.c | 5 ++--
net/ipv4/tcp_highspeed.c | 4 +--
net/ipv4/tcp_htcp.c | 4 +--
net/ipv4/tcp_hybla.c | 5 ++--
net/ipv4/tcp_illinois.c | 5 ++--
net/ipv4/tcp_input.c | 6 ++---
net/ipv4/tcp_lp.c | 5 ++--
net/ipv4/tcp_scalable.c | 5 ++--
net/ipv4/tcp_vegas.c | 11 ++++----
net/ipv4/tcp_veno.c | 9 ++++---
net/ipv4/tcp_yeah.c | 5 ++--
16 files changed, 59 insertions(+), 82 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 6c00983..8b8a057 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -267,17 +267,6 @@ tcp_max_orphans - INTEGER
more aggressively. Let me to remind again: each orphan eats
up to ~64K of unswappable memory.
-tcp_max_ssthresh - INTEGER
- Limited Slow-Start for TCP with large congestion windows (cwnd) defined in
- RFC3742. Limited slow-start is a mechanism to limit growth of the cwnd
- on the region where cwnd is larger than tcp_max_ssthresh. TCP increases cwnd
- by at most tcp_max_ssthresh segments, and by at least tcp_max_ssthresh/2
- segments per RTT when the cwnd is above tcp_max_ssthresh.
- If TCP connection increased cwnd to thousands (or tens of thousands) segments,
- and thousands of packets were being dropped during slow-start, you can set
- tcp_max_ssthresh to improve performance for new TCP connection.
- Default: 0 (off)
-
tcp_max_syn_backlog - INTEGER
Maximal number of remembered connection requests, which have not
received an acknowledgment from connecting client.
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 2d7b4bd..70e55d2 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -275,7 +275,6 @@ extern int sysctl_tcp_mtu_probing;
extern int sysctl_tcp_base_mss;
extern int sysctl_tcp_workaround_signed_windows;
extern int sysctl_tcp_slow_start_after_idle;
-extern int sysctl_tcp_max_ssthresh;
extern int sysctl_tcp_thin_linear_timeouts;
extern int sysctl_tcp_thin_dupack;
extern int sysctl_tcp_early_retrans;
@@ -797,7 +796,7 @@ struct tcp_congestion_ops {
/* lower bound for congestion window (optional) */
u32 (*min_cwnd)(const struct sock *sk);
/* do new cwnd calculation (required) */
- void (*cong_avoid)(struct sock *sk, u32 ack, u32 in_flight);
+ void (*cong_avoid)(struct sock *sk, u32 ack, u32 acked, u32 in_flight);
/* call before changing ca_state (optional) */
void (*set_state)(struct sock *sk, u8 new_state);
/* call when cwnd event occurs (optional) */
@@ -824,12 +823,12 @@ void tcp_get_available_congestion_control(char *buf, size_t len);
void tcp_get_allowed_congestion_control(char *buf, size_t len);
int tcp_set_allowed_congestion_control(char *allowed);
int tcp_set_congestion_control(struct sock *sk, const char *name);
-void tcp_slow_start(struct tcp_sock *tp);
+int tcp_slow_start(struct tcp_sock *tp, u32 acked);
void tcp_cong_avoid_ai(struct tcp_sock *tp, u32 w);
extern struct tcp_congestion_ops tcp_init_congestion_ops;
u32 tcp_reno_ssthresh(struct sock *sk);
-void tcp_reno_cong_avoid(struct sock *sk, u32 ack, u32 in_flight);
+void tcp_reno_cong_avoid(struct sock *sk, u32 ack, u32 acked, u32 in_flight);
u32 tcp_reno_min_cwnd(const struct sock *sk);
extern struct tcp_congestion_ops tcp_reno;
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index d5b1390..3d69ec8 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -701,13 +701,6 @@ static struct ctl_table ipv4_table[] = {
.proc_handler = proc_allowed_congestion_control,
},
{
- .procname = "tcp_max_ssthresh",
- .data = &sysctl_tcp_max_ssthresh,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = proc_dointvec,
- },
- {
.procname = "tcp_thin_linear_timeouts",
.data = &sysctl_tcp_thin_linear_timeouts,
.maxlen = sizeof(int),
diff --git a/net/ipv4/tcp_bic.c b/net/ipv4/tcp_bic.c
index f45e1c2..821846f 100644
--- a/net/ipv4/tcp_bic.c
+++ b/net/ipv4/tcp_bic.c
@@ -140,7 +140,8 @@ static inline void bictcp_update(struct bictcp *ca, u32 cwnd)
ca->cnt = 1;
}
-static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+ u32 in_flight)
{
struct tcp_sock *tp = tcp_sk(sk);
struct bictcp *ca = inet_csk_ca(sk);
@@ -149,7 +150,7 @@ static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
return;
if (tp->snd_cwnd <= tp->snd_ssthresh)
- tcp_slow_start(tp);
+ tcp_slow_start(tp, acked);
else {
bictcp_update(ca, tp->snd_cwnd);
tcp_cong_avoid_ai(tp, ca->cnt);
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
index 019c238..ad37bf1 100644
--- a/net/ipv4/tcp_cong.c
+++ b/net/ipv4/tcp_cong.c
@@ -15,8 +15,6 @@
#include <linux/gfp.h>
#include <net/tcp.h>
-int sysctl_tcp_max_ssthresh = 0;
-
static DEFINE_SPINLOCK(tcp_cong_list_lock);
static LIST_HEAD(tcp_cong_list);
@@ -299,35 +297,24 @@ bool tcp_is_cwnd_limited(const struct sock *sk, u32 in_flight)
}
EXPORT_SYMBOL_GPL(tcp_is_cwnd_limited);
-/*
- * Slow start is used when congestion window is less than slow start
- * threshold. This version implements the basic RFC2581 version
- * and optionally supports:
- * RFC3742 Limited Slow Start - growth limited to max_ssthresh
- * RFC3465 Appropriate Byte Counting - growth limited by bytes acknowledged
+/* Slow start is used when congestion window is no greater than the slow start
+ * threshold. We base on RFC2581 and also handle stretch ACKs properly.
+ * We do not implement RFC3465 Appropriate Byte Counting (ABC) per se but
+ * something better;) a packet is only considered (s)acked in its entirety to
+ * defend the ACK attacks described in the RFC. Slow start processes a stretch
+ * ACK of degree N as if N acks of degree 1 are received back to back except
+ * ABC caps N to 2. Slow start exits when cwnd grows over ssthresh and
+ * returns the leftover acks to adjust cwnd in congestion avoidance mode.
*/
-void tcp_slow_start(struct tcp_sock *tp)
+int tcp_slow_start(struct tcp_sock *tp, u32 acked)
{
- int cnt; /* increase in packets */
- unsigned int delta = 0;
- u32 snd_cwnd = tp->snd_cwnd;
-
- if (unlikely(!snd_cwnd)) {
- pr_err_once("snd_cwnd is nul, please report this bug.\n");
- snd_cwnd = 1U;
- }
+ u32 cwnd = tp->snd_cwnd + acked;
- if (sysctl_tcp_max_ssthresh > 0 && tp->snd_cwnd > sysctl_tcp_max_ssthresh)
- cnt = sysctl_tcp_max_ssthresh >> 1; /* limited slow start */
- else
- cnt = snd_cwnd; /* exponential increase */
-
- tp->snd_cwnd_cnt += cnt;
- while (tp->snd_cwnd_cnt >= snd_cwnd) {
- tp->snd_cwnd_cnt -= snd_cwnd;
- delta++;
- }
- tp->snd_cwnd = min(snd_cwnd + delta, tp->snd_cwnd_clamp);
+ if (cwnd > tp->snd_ssthresh)
+ cwnd = tp->snd_ssthresh + 1;
+ acked -= cwnd - tp->snd_cwnd;
+ tp->snd_cwnd = min(cwnd, tp->snd_cwnd_clamp);
+ return acked;
}
EXPORT_SYMBOL_GPL(tcp_slow_start);
@@ -351,7 +338,7 @@ EXPORT_SYMBOL_GPL(tcp_cong_avoid_ai);
/* This is Jacobson's slow start and congestion avoidance.
* SIGCOMM '88, p. 328.
*/
-void tcp_reno_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+void tcp_reno_cong_avoid(struct sock *sk, u32 ack, u32 acked, u32 in_flight)
{
struct tcp_sock *tp = tcp_sk(sk);
@@ -360,7 +347,7 @@ void tcp_reno_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
/* In "safe" area, increase. */
if (tp->snd_cwnd <= tp->snd_ssthresh)
- tcp_slow_start(tp);
+ tcp_slow_start(tp, acked);
/* In dangerous area, increase slowly. */
else
tcp_cong_avoid_ai(tp, tp->snd_cwnd);
diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
index b6ae92a..828e4c3 100644
--- a/net/ipv4/tcp_cubic.c
+++ b/net/ipv4/tcp_cubic.c
@@ -304,7 +304,8 @@ static inline void bictcp_update(struct bictcp *ca, u32 cwnd)
ca->cnt = 1;
}
-static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+ u32 in_flight)
{
struct tcp_sock *tp = tcp_sk(sk);
struct bictcp *ca = inet_csk_ca(sk);
@@ -315,7 +316,7 @@ static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
if (tp->snd_cwnd <= tp->snd_ssthresh) {
if (hystart && after(ack, ca->end_seq))
bictcp_hystart_reset(sk);
- tcp_slow_start(tp);
+ tcp_slow_start(tp, acked);
} else {
bictcp_update(ca, tp->snd_cwnd);
tcp_cong_avoid_ai(tp, ca->cnt);
diff --git a/net/ipv4/tcp_highspeed.c b/net/ipv4/tcp_highspeed.c
index 30f27f6..8ed9305 100644
--- a/net/ipv4/tcp_highspeed.c
+++ b/net/ipv4/tcp_highspeed.c
@@ -109,7 +109,7 @@ static void hstcp_init(struct sock *sk)
tp->snd_cwnd_clamp = min_t(u32, tp->snd_cwnd_clamp, 0xffffffff/128);
}
-static void hstcp_cong_avoid(struct sock *sk, u32 adk, u32 in_flight)
+static void hstcp_cong_avoid(struct sock *sk, u32 ack, u32 acked, u32 in_flight)
{
struct tcp_sock *tp = tcp_sk(sk);
struct hstcp *ca = inet_csk_ca(sk);
@@ -118,7 +118,7 @@ static void hstcp_cong_avoid(struct sock *sk, u32 adk, u32 in_flight)
return;
if (tp->snd_cwnd <= tp->snd_ssthresh)
- tcp_slow_start(tp);
+ tcp_slow_start(tp, acked);
else {
/* Update AIMD parameters.
*
diff --git a/net/ipv4/tcp_htcp.c b/net/ipv4/tcp_htcp.c
index c1a8175..4a194ac 100644
--- a/net/ipv4/tcp_htcp.c
+++ b/net/ipv4/tcp_htcp.c
@@ -227,7 +227,7 @@ static u32 htcp_recalc_ssthresh(struct sock *sk)
return max((tp->snd_cwnd * ca->beta) >> 7, 2U);
}
-static void htcp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void htcp_cong_avoid(struct sock *sk, u32 ack, u32 acked, u32 in_flight)
{
struct tcp_sock *tp = tcp_sk(sk);
struct htcp *ca = inet_csk_ca(sk);
@@ -236,7 +236,7 @@ static void htcp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
return;
if (tp->snd_cwnd <= tp->snd_ssthresh)
- tcp_slow_start(tp);
+ tcp_slow_start(tp, acked);
else {
/* In dangerous area, increase slowly.
* In theory this is tp->snd_cwnd += alpha / tp->snd_cwnd
diff --git a/net/ipv4/tcp_hybla.c b/net/ipv4/tcp_hybla.c
index 57bdd17..478fe82 100644
--- a/net/ipv4/tcp_hybla.c
+++ b/net/ipv4/tcp_hybla.c
@@ -85,7 +85,8 @@ static inline u32 hybla_fraction(u32 odds)
* o Give cwnd a new value based on the model proposed
* o remember increments <1
*/
-static void hybla_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void hybla_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+ u32 in_flight)
{
struct tcp_sock *tp = tcp_sk(sk);
struct hybla *ca = inet_csk_ca(sk);
@@ -102,7 +103,7 @@ static void hybla_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
return;
if (!ca->hybla_en) {
- tcp_reno_cong_avoid(sk, ack, in_flight);
+ tcp_reno_cong_avoid(sk, ack, acked, in_flight);
return;
}
diff --git a/net/ipv4/tcp_illinois.c b/net/ipv4/tcp_illinois.c
index 834857f..8a52099 100644
--- a/net/ipv4/tcp_illinois.c
+++ b/net/ipv4/tcp_illinois.c
@@ -256,7 +256,8 @@ static void tcp_illinois_state(struct sock *sk, u8 new_state)
/*
* Increase window in response to successful acknowledgment.
*/
-static void tcp_illinois_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void tcp_illinois_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+ u32 in_flight)
{
struct tcp_sock *tp = tcp_sk(sk);
struct illinois *ca = inet_csk_ca(sk);
@@ -270,7 +271,7 @@ static void tcp_illinois_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
/* In slow start */
if (tp->snd_cwnd <= tp->snd_ssthresh)
- tcp_slow_start(tp);
+ tcp_slow_start(tp, acked);
else {
u32 delta;
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index b935397..ad8d869 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2928,10 +2928,10 @@ static void tcp_synack_rtt_meas(struct sock *sk, struct request_sock *req)
tcp_ack_update_rtt(sk, FLAG_SYN_ACKED, seq_rtt, -1);
}
-static void tcp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void tcp_cong_avoid(struct sock *sk, u32 ack, u32 acked, u32 in_flight)
{
const struct inet_connection_sock *icsk = inet_csk(sk);
- icsk->icsk_ca_ops->cong_avoid(sk, ack, in_flight);
+ icsk->icsk_ca_ops->cong_avoid(sk, ack, acked, in_flight);
tcp_sk(sk)->snd_cwnd_stamp = tcp_time_stamp;
}
@@ -3441,7 +3441,7 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
/* Advance cwnd if state allows */
if (tcp_may_raise_cwnd(sk, flag))
- tcp_cong_avoid(sk, ack, prior_in_flight);
+ tcp_cong_avoid(sk, ack, acked, prior_in_flight);
if (tcp_ack_is_dubious(sk, flag)) {
is_dupack = !(flag & (FLAG_SND_UNA_ADVANCED | FLAG_NOT_DUP));
diff --git a/net/ipv4/tcp_lp.c b/net/ipv4/tcp_lp.c
index 72f7218..991d62a 100644
--- a/net/ipv4/tcp_lp.c
+++ b/net/ipv4/tcp_lp.c
@@ -115,12 +115,13 @@ static void tcp_lp_init(struct sock *sk)
* Will only call newReno CA when away from inference.
* From TCP-LP's paper, this will be handled in additive increasement.
*/
-static void tcp_lp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void tcp_lp_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+ u32 in_flight)
{
struct lp *lp = inet_csk_ca(sk);
if (!(lp->flag & LP_WITHIN_INF))
- tcp_reno_cong_avoid(sk, ack, in_flight);
+ tcp_reno_cong_avoid(sk, ack, acked, in_flight);
}
/**
diff --git a/net/ipv4/tcp_scalable.c b/net/ipv4/tcp_scalable.c
index 8ce55b8..19ea6c2 100644
--- a/net/ipv4/tcp_scalable.c
+++ b/net/ipv4/tcp_scalable.c
@@ -15,7 +15,8 @@
#define TCP_SCALABLE_AI_CNT 50U
#define TCP_SCALABLE_MD_SCALE 3
-static void tcp_scalable_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void tcp_scalable_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+ u32 in_flight)
{
struct tcp_sock *tp = tcp_sk(sk);
@@ -23,7 +24,7 @@ static void tcp_scalable_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
return;
if (tp->snd_cwnd <= tp->snd_ssthresh)
- tcp_slow_start(tp);
+ tcp_slow_start(tp, acked);
else
tcp_cong_avoid_ai(tp, min(tp->snd_cwnd, TCP_SCALABLE_AI_CNT));
}
diff --git a/net/ipv4/tcp_vegas.c b/net/ipv4/tcp_vegas.c
index 80fa2bf..06cae62 100644
--- a/net/ipv4/tcp_vegas.c
+++ b/net/ipv4/tcp_vegas.c
@@ -163,13 +163,14 @@ static inline u32 tcp_vegas_ssthresh(struct tcp_sock *tp)
return min(tp->snd_ssthresh, tp->snd_cwnd-1);
}
-static void tcp_vegas_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void tcp_vegas_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+ u32 in_flight)
{
struct tcp_sock *tp = tcp_sk(sk);
struct vegas *vegas = inet_csk_ca(sk);
if (!vegas->doing_vegas_now) {
- tcp_reno_cong_avoid(sk, ack, in_flight);
+ tcp_reno_cong_avoid(sk, ack, acked, in_flight);
return;
}
@@ -194,7 +195,7 @@ static void tcp_vegas_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
/* We don't have enough RTT samples to do the Vegas
* calculation, so we'll behave like Reno.
*/
- tcp_reno_cong_avoid(sk, ack, in_flight);
+ tcp_reno_cong_avoid(sk, ack, acked, in_flight);
} else {
u32 rtt, diff;
u64 target_cwnd;
@@ -243,7 +244,7 @@ static void tcp_vegas_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
} else if (tp->snd_cwnd <= tp->snd_ssthresh) {
/* Slow start. */
- tcp_slow_start(tp);
+ tcp_slow_start(tp, acked);
} else {
/* Congestion avoidance. */
@@ -283,7 +284,7 @@ static void tcp_vegas_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
}
/* Use normal slow start */
else if (tp->snd_cwnd <= tp->snd_ssthresh)
- tcp_slow_start(tp);
+ tcp_slow_start(tp, acked);
}
diff --git a/net/ipv4/tcp_veno.c b/net/ipv4/tcp_veno.c
index ac43cd7..326475a 100644
--- a/net/ipv4/tcp_veno.c
+++ b/net/ipv4/tcp_veno.c
@@ -114,13 +114,14 @@ static void tcp_veno_cwnd_event(struct sock *sk, enum tcp_ca_event event)
tcp_veno_init(sk);
}
-static void tcp_veno_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void tcp_veno_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+ u32 in_flight)
{
struct tcp_sock *tp = tcp_sk(sk);
struct veno *veno = inet_csk_ca(sk);
if (!veno->doing_veno_now) {
- tcp_reno_cong_avoid(sk, ack, in_flight);
+ tcp_reno_cong_avoid(sk, ack, acked, in_flight);
return;
}
@@ -133,7 +134,7 @@ static void tcp_veno_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
/* We don't have enough rtt samples to do the Veno
* calculation, so we'll behave like Reno.
*/
- tcp_reno_cong_avoid(sk, ack, in_flight);
+ tcp_reno_cong_avoid(sk, ack, acked, in_flight);
} else {
u64 target_cwnd;
u32 rtt;
@@ -152,7 +153,7 @@ static void tcp_veno_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
if (tp->snd_cwnd <= tp->snd_ssthresh) {
/* Slow start. */
- tcp_slow_start(tp);
+ tcp_slow_start(tp, acked);
} else {
/* Congestion avoidance. */
if (veno->diff < beta) {
diff --git a/net/ipv4/tcp_yeah.c b/net/ipv4/tcp_yeah.c
index 05c3b6f..a347a07 100644
--- a/net/ipv4/tcp_yeah.c
+++ b/net/ipv4/tcp_yeah.c
@@ -69,7 +69,8 @@ static void tcp_yeah_pkts_acked(struct sock *sk, u32 pkts_acked, s32 rtt_us)
tcp_vegas_pkts_acked(sk, pkts_acked, rtt_us);
}
-static void tcp_yeah_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void tcp_yeah_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+ u32 in_flight)
{
struct tcp_sock *tp = tcp_sk(sk);
struct yeah *yeah = inet_csk_ca(sk);
@@ -78,7 +79,7 @@ static void tcp_yeah_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
return;
if (tp->snd_cwnd <= tp->snd_ssthresh)
- tcp_slow_start(tp);
+ tcp_slow_start(tp, acked);
else if (!yeah->doing_reno_now) {
/* Scalable */
--
1.8.4.1
^ permalink raw reply related
* Re: [PATCH] x86: Run checksumming in parallel accross multiple alu's
From: Neil Horman @ 2013-10-31 18:30 UTC (permalink / raw)
To: Doug Ledford
Cc: Ingo Molnar, Eric Dumazet, linux-kernel, netdev, David Laight
In-Reply-To: <52710B09.6090302@redhat.com>
On Wed, Oct 30, 2013 at 09:35:05AM -0400, Doug Ledford wrote:
> On 10/30/2013 07:02 AM, Neil Horman wrote:
>
> >That does makes sense, but it then begs the question, whats the advantage of
> >having multiple alu's at all?
>
> There's lots of ALU operations that don't operate on the flags or
> other entities that can be run in parallel.
>
> >If they're just going to serialize on the
> >updating of the condition register, there doesn't seem to be much advantage in
> >having multiple alu's at all, especially if a common use case (parallelizing an
> >operation on a large linear dataset) resulted in lower performance.
> >
> >/me wonders if rearranging the instructions into this order:
> >adcq 0*8(src), res1
> >adcq 1*8(src), res2
> >adcq 2*8(src), res1
> >
> >would prevent pipeline stalls. That would be interesting data, and (I think)
> >support your theory, Doug. I'll give that a try
>
> Just to avoid spending too much time on various combinations, here
> are the methods I've tried:
>
> Original code
> 2 chains doing interleaved memory accesses
> 2 chains doing serial memory accesses (as above)
> 4 chains doing serial memory accesses
> 4 chains using 32bit values in 64bit registers so you can always use
> add instead of adc and never need the carry flag
>
> And I've done all of the above with simple prefetch and smart prefetch.
>
>
So, above and beyond this I spent yesterday trying this pattern, something Doug
and I discussed together offline:
asm("prefetch 5*64(%[src])\n\t"
"addq 0*8(%[src]),%[res1]\n\t"
"jo 2f\n\t"
"incq %[cry]\n\t"
"2:addq 1*8(%[src]),%[res2]\n\t"
"jc 3f\n\t"
"incq %[cry]\n\t"
"3:addq 2*8(%[src]),%[res1]\n\t"
...
The hope being that by using the add instead instead of the adc instruction, and
alternatively testing the overflow and carry flags, I could break the
serialization on the flags register between subeuqent adds and start doing
things in parallel (creating a poor mans adcx/adox instruction in effect). It
functions, but unfortunately the performance lost to the completely broken
branch prediction that this inflicts makes it a non starter:
Base Performance:
Performance counter stats for './test.sh' (20 runs):
48,143,372 L1-dcache-load-misses ( +- 0.03% ) [74.99%]
0 L1-dcache-prefetches [75.00%]
13,913,339,911 cycles # 0.000 GHz ( +- 0.06% ) [75.01%]
28,878,999 branch-misses ( +- 0.05% ) [75.00%]
5.367618727 seconds time elapsed ( +- 0.06% )
Prefetch and simluated adcx/adox from above:
Performance counter stats for './test.sh' (20 runs):
35,704,331 L1-dcache-load-misses ( +- 0.07% ) [75.00%]
0 L1-dcache-prefetches [75.00%]
19,751,409,264 cycles # 0.000 GHz ( +- 0.59% ) [75.00%]
34,850,056 branch-misses ( +- 1.29% ) [75.00%]
7.768602160 seconds time elapsed ( +- 1.38% )
With the above instruction changes the prefetching lowers our dcache miss rate
significantly, but greatly raises our branch miss rate, and absolutely kills our
cycle count and run time.
At this point I feel like this is dead in the water. I apologize for wasting
everyones time. The best thing to do here would seem to be:
1) Add in some prefetching (from what I've seen a simple prefetch is as
performant as smart prefetching), so we may as well do it exactly as
csum_copy_from_user does it, and save ourselves the extra while loop.
2) Revisit this when the AVX extensions, or the adcx/adox instructions are
available and we can really preform parallel alu ops here.
Does that sound reasonable?
Neil
^ permalink raw reply
* Good Day
From: BANK, OF, khairiya.masoud @ 2013-10-30 17:09 UTC (permalink / raw)
To: Recipients
Good Day,
My name is Mr. Lee from Lars Jayden Limited, Thailand. i am interested in
purchasing your product, can you send me your latest product photo and
catalog. also send me your minimum order quantity and price list. hope to
hear from you soon.
Regards.
Purchase manager
Mr. Lee.
Address: Office No-57
cambride colony. Thailand
^ permalink raw reply
* Re: [gpio:for-next 67/67] pch_gbe_main.c:undefined reference to `devm_gpio_request_one'
From: David Cohen @ 2013-10-31 19:12 UTC (permalink / raw)
To: Linus Walleij, Darren Hart
Cc: David S. Miller, netdev@vger.kernel.org, Fengguang Wu,
Alexandre Courbot, linux-gpio@vger.kernel.org
In-Reply-To: <CACRpkdZM8BBR6NAUTuSpEErZe2541-CvuwFSe13FDfxOBnnygA@mail.gmail.com>
On 10/30/2013 11:21 AM, Linus Walleij wrote:
> On Sat, Oct 26, 2013 at 10:31 PM, David Cohen
> <david.a.cohen@linux.intel.com> wrote:
>
>> Maybe if GPIOLIB has the static inline stubs returning -ENODEV we could
>> use a patch similar to the one attached here.
>
> I've added stubs returning -EINVAL just like all siblings there, if you
> like to have this changed, send a patch changing it to -ENODEV
> and this patch on top as 2/2 please.
Thanks.
Darren, do you have any objections WRT failing pch_gbe on Minnowboard
due to lack of GPIO?
BR, David
^ permalink raw reply
* Re: [Xen-devel] [PATCH net-next RFC 1/5] xen-netback: Introduce TX grant map definitions
From: Zoltan Kiss @ 2013-10-31 19:22 UTC (permalink / raw)
To: Jan Beulich
Cc: ian.campbell, jonathan.davies, wei.liu2, xen-devel, linux-kernel,
netdev
In-Reply-To: <5270DF3602000078000FDFBE@nat28.tlf.novell.com>
On 30/10/13 09:28, Jan Beulich wrote:
>>>> On 30.10.13 at 01:50, Zoltan Kiss <zoltan.kiss@citrix.com> wrote:
>> @@ -119,13 +126,22 @@ struct xenvif {
>> pending_ring_idx_t pending_cons;
>> u16 pending_ring[MAX_PENDING_REQS];
>> struct pending_tx_info pending_tx_info[MAX_PENDING_REQS];
>> + grant_handle_t grant_tx_handle[MAX_PENDING_REQS];
>>
>> /* Coalescing tx requests before copying makes number of grant
>> * copy ops greater or equal to number of slots required. In
>> * worst case a tx request consumes 2 gnttab_copy.
>> */
>> struct gnttab_copy tx_copy_ops[2*MAX_PENDING_REQS];
>> + struct gnttab_unmap_grant_ref tx_unmap_ops[MAX_PENDING_REQS];
>> + struct gnttab_map_grant_ref tx_map_ops[MAX_PENDING_REQS];
>> + /* passed to gnttab_[un]map_refs with pages under (un)mapping */
>> + struct page *pages_to_gnt[MAX_PENDING_REQS];
>
> I think you will want to try to limit the structure size here by putting
> things into unions that can't be used at the same time: Without
> having looked at the later patches yet, it seems quite unlikely that
> map and unmap can be used simultaneously. And the total of copy
> and map can't also be used at the same time, as for each pending
> request you would use either up to two copy slots or a single map
> slot. I didn't look for further opportunities of sharing space.
Indeed, map and unmap can't be done at the same time, so it's safe to
put them into union. But I'm afraid grant_tx_handle and pages_to_gnt
can't share space with other members.
tx_copy_ops is a different topic, let's discuss that in it's own thread ...
Thanks,
Zoli
^ permalink raw reply
* [PATCH 1/2] can: c_can: Fix RX message handling, handle lost message before EOB
From: Marc Kleine-Budde @ 2013-10-31 19:24 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, Markus Pargmann, linux-stable,
Marc Kleine-Budde
In-Reply-To: <1383247490-26984-1-git-send-email-mkl@pengutronix.de>
From: Markus Pargmann <mpa@pengutronix.de>
If we handle end of block messages with higher priority than a lost message,
we can run into an endless interrupt loop.
This is reproducable with a am335x processor and "cansequence -r" at 1Mbit.
As soon as we loose a packet we can't escape from an interrupt loop.
This patch fixes the problem by handling lost packets before EOB packets.
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/c_can/c_can.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index a668cd4..e3fc07c 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -814,9 +814,6 @@ static int c_can_do_rx_poll(struct net_device *dev, int quota)
msg_ctrl_save = priv->read_reg(priv,
C_CAN_IFACE(MSGCTRL_REG, 0));
- if (msg_ctrl_save & IF_MCONT_EOB)
- return num_rx_pkts;
-
if (msg_ctrl_save & IF_MCONT_MSGLST) {
c_can_handle_lost_msg_obj(dev, 0, msg_obj);
num_rx_pkts++;
@@ -824,6 +821,9 @@ static int c_can_do_rx_poll(struct net_device *dev, int quota)
continue;
}
+ if (msg_ctrl_save & IF_MCONT_EOB)
+ return num_rx_pkts;
+
if (!(msg_ctrl_save & IF_MCONT_NEWDAT))
continue;
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH 2/2] can: kvaser_usb: fix usb endpoints detection
From: Marc Kleine-Budde @ 2013-10-31 19:24 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, Olivier Sobrie, linux-stable,
Marc Kleine-Budde
In-Reply-To: <1383247490-26984-1-git-send-email-mkl@pengutronix.de>
From: Olivier Sobrie <olivier@sobrie.be>
Some devices, like the Kvaser Memorator Professional, have several bulk in
endpoints. Only the first one found must be used by the driver. The same holds
for the bulk out endpoint. The official Kvaser driver (leaf) was used as
reference for this patch.
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/usb/kvaser_usb.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 3b95465..4b2d5ed 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1544,9 +1544,9 @@ static int kvaser_usb_init_one(struct usb_interface *intf,
return 0;
}
-static void kvaser_usb_get_endpoints(const struct usb_interface *intf,
- struct usb_endpoint_descriptor **in,
- struct usb_endpoint_descriptor **out)
+static int kvaser_usb_get_endpoints(const struct usb_interface *intf,
+ struct usb_endpoint_descriptor **in,
+ struct usb_endpoint_descriptor **out)
{
const struct usb_host_interface *iface_desc;
struct usb_endpoint_descriptor *endpoint;
@@ -1557,12 +1557,18 @@ static void kvaser_usb_get_endpoints(const struct usb_interface *intf,
for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
endpoint = &iface_desc->endpoint[i].desc;
- if (usb_endpoint_is_bulk_in(endpoint))
+ if (!*in && usb_endpoint_is_bulk_in(endpoint))
*in = endpoint;
- if (usb_endpoint_is_bulk_out(endpoint))
+ if (!*out && usb_endpoint_is_bulk_out(endpoint))
*out = endpoint;
+
+ /* use first bulk endpoint for in and out */
+ if (*in && *out)
+ return 0;
}
+
+ return -ENODEV;
}
static int kvaser_usb_probe(struct usb_interface *intf,
@@ -1576,8 +1582,8 @@ static int kvaser_usb_probe(struct usb_interface *intf,
if (!dev)
return -ENOMEM;
- kvaser_usb_get_endpoints(intf, &dev->bulk_in, &dev->bulk_out);
- if (!dev->bulk_in || !dev->bulk_out) {
+ err = kvaser_usb_get_endpoints(intf, &dev->bulk_in, &dev->bulk_out);
+ if (err) {
dev_err(&intf->dev, "Cannot get usb endpoint(s)");
return err;
}
--
1.8.4.rc3
^ permalink raw reply related
* pull-request: can 2013-10-31
From: Marc Kleine-Budde @ 2013-10-31 19:24 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel
Hello David,
I have two late fixes for the v3.12 release:
The first patch fixes a problem in the c_can's RX message handling, which can
lead to an endless interrupt loop under heavy load if messages are lost. The
second patch is by Olivier Sobrie and fixes the endpoint detection of the
kvaser_usb driver, which is needed for some devices.
regards,
Marc
---
The following changes since commit c17cb8b55b104c549aa20a72fa44141ad2c65ec2:
doc:net: Fix typo in Documentation/networking (2013-10-30 17:10:20 -0400)
are available in the git repository at:
git://gitorious.org/linux-can/linux-can.git fixes-for-3.12
for you to fetch changes up to 896e23bd04ea50a146dffd342e2f96180f0812a5:
can: kvaser_usb: fix usb endpoints detection (2013-10-31 20:20:23 +0100)
----------------------------------------------------------------
Markus Pargmann (1):
can: c_can: Fix RX message handling, handle lost message before EOB
Olivier Sobrie (1):
can: kvaser_usb: fix usb endpoints detection
drivers/net/can/c_can/c_can.c | 6 +++---
drivers/net/can/usb/kvaser_usb.c | 20 +++++++++++++-------
2 files changed, 16 insertions(+), 10 deletions(-)
^ permalink raw reply
* Re: [PATCH net-next RFC 1/5] xen-netback: Introduce TX grant map definitions
From: Zoltan Kiss @ 2013-10-31 19:33 UTC (permalink / raw)
To: Zoltan Kiss, ian.campbell, wei.liu2, xen-devel, netdev,
linux-kernel, jonathan.davies
In-Reply-To: <1383094220-14775-2-git-send-email-zoltan.kiss@citrix.com>
On 30/10/13 00:50, Zoltan Kiss wrote:
> +void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success)
> +{
> + unsigned long flags;
> + pending_ring_idx_t index;
> + u16 pending_idx = ubuf->desc;
> + struct pending_tx_info *temp =
> + container_of(ubuf, struct pending_tx_info, callback_struct);
> + struct xenvif *vif =
> + container_of(temp - pending_idx, struct xenvif,
> + pending_tx_info[0]);
> +
> + spin_lock_irqsave(&vif->dealloc_lock, flags);
> + do {
> + pending_idx = ubuf->desc;
> + ubuf = (struct ubuf_info *) ubuf->ctx;
> + index = pending_index(vif->dealloc_prod);
> + vif->dealloc_ring[index] = pending_idx;
> + /* Sync with xenvif_tx_action_dealloc:
> + * insert idx then incr producer.
> + */
> + smp_wmb();
> + vif->dealloc_prod++;
> + napi_schedule(&vif->napi);
> + } while (ubuf);
> + spin_unlock_irqrestore(&vif->dealloc_lock, flags);
> +}
Another possible place for improvement is the placing of napi_schedule.
Now it get called after every fragment, which is probably suboptimal. I
think it's likely that the vif thread can't finish one dealloc faster
than one iteration of this while loop.
Another idea is to place it after the while, so it get called only once,
but in the meantime the thread doesn't have chance to start working on
the deallocs.
A compromise might be to do it once in the first iteration of the loop,
and then once after the loop to make sure the thread knows about the
dealloc.
Thoughts?
Zoli
^ permalink raw reply
* Re: [Xen-devel] [PATCH net-next RFC 3/5] xen-netback: Remove old TX grant copy definitons
From: Zoltan Kiss @ 2013-10-31 19:46 UTC (permalink / raw)
To: Jan Beulich
Cc: ian.campbell, jonathan.davies, wei.liu2, xen-devel, linux-kernel,
netdev
In-Reply-To: <5270E1F902000078000FDFCA@nat28.tlf.novell.com>
On 30/10/13 09:39, Jan Beulich wrote:
>>>> On 30.10.13 at 01:50, Zoltan Kiss <zoltan.kiss@citrix.com> wrote:
>> These became obsolate with grant mapping.
>
> I didn't look at this in detail, but I'm surprised you can get away
> without any copying: For one, the header part needs copying
> anyway, so you'd pointlessly map and then copy it if it's small
> enough.
Yep, that's a further plan for optimization. I think I will add that as
a separate patch to the series later. But that doesn't necessarily needs
these definitions, let's see that later.
> And second you need to be prepared for the frontend
> to hand you more slots than you can fit in MAX_SKB_FRAGS
> (namely when MAX_SKB_FRAGS < XEN_NETIF_NR_SLOTS_MIN),
> which you can't handle with mapping alone afaict.
Oh, I was not aware of this problem. And indeed, the trivial solution is
to keep the grant copy methods for such kind of packets, however that
sounds quite nasty.
My another idea is to use skb_shinfo(skb)->frag_list for such packets,
so the stack will see it as a fragmented IP packet. It might be less
efficient than coalescing them into one skb during grant copy at first
place, but probably a cleaner solution. If we don't care that much about
the performance of such guests, it might be a better solution.
But I don't know that closely the IP fragmentation ideas, so it might be
a bad idea. I'm happy to hear comments from people who have more
experience with that.
Zoli
^ permalink raw reply
* Re: [PATCH net-next RFC 4/5] xen-netback: Fix indentations
From: Zoltan Kiss @ 2013-10-31 19:48 UTC (permalink / raw)
To: Wei Liu; +Cc: ian.campbell, xen-devel, netdev, linux-kernel, jonathan.davies
In-Reply-To: <20131030111312.GN5221@zion.uk.xensource.com>
On 30/10/13 11:13, Wei Liu wrote:
> On Wed, Oct 30, 2013 at 12:50:19AM +0000, Zoltan Kiss wrote:
>> I've left intentionally these indentations in this way, to improve readability of previous patches.
>>
>
> Apparently this doesn't deserve a single patch -- please squash it when
> you post non-RFC series.
>
> Wei.
>
Yep, I just want to keep them separately until the series get it's final
form.
Zoli
^ permalink raw reply
* pull-request: can-next 2013-10-31
From: Marc Kleine-Budde @ 2013-10-31 20:06 UTC (permalink / raw)
To: netdev; +Cc: David Miller, linux-can@vger.kernel.org, kernel@pengutronix.de
[-- Attachment #1: Type: text/plain, Size: 1706 bytes --]
Hello David,
here's a pull request for net-next.
It includes a patch by Oliver Hartkopp et al. that adds documentation
for the broadcast manager to Documentation/networking/can.txt. Three
patches by me that clean up the netlink handling code in the CAN core.
And another patch that removes a not needed function from the ti_hecc
driver.
regards,
Marc
---
The following changes since commit ba4865027c11d7ac8e5a33e0624dd415caab2027:
ipv6: remove the unnecessary statement in find_match() (2013-10-30 17:06:51 -0400)
are available in the git repository at:
git://gitorious.org/linux-can/linux-can-next.git for-davem
for you to fetch changes up to 26896fd98130cdda73b19c75849a0d25cb850cb8:
can: ti_hecc: remove priv->can.do_get_state() callback (2013-10-31 20:55:16 +0100)
----------------------------------------------------------------
Marc Kleine-Budde (4):
can: dev: sort can_get_size() by IFLA_CAN_*
can: dev: sort can_fill_info() by IFLA_CAN_*
can: dev: sort can_changelink() by IFLA_CAN_*
can: ti_hecc: remove priv->can.do_get_state() callback
Oliver Hartkopp (1):
can: add broadcast manager documentation
Documentation/networking/can.txt | 217 +++++++++++++++++++++++++++++++++++++++
drivers/net/can/dev.c | 67 ++++++------
drivers/net/can/ti_hecc.c | 10 --
3 files changed, 249 insertions(+), 45 deletions(-)
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply
* Re: [net-next PATCH] net: codel: Avoid undefined behavior from signed overflow
From: Jesper Dangaard Brouer @ 2013-10-31 20:40 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Jesper Dangaard Brouer, netdev, Paul E. McKenney, Dave Taht
In-Reply-To: <1383232241.4857.73.camel@edumazet-glaptop.roam.corp.google.com>
On Thu, 31 Oct 2013 08:10:41 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2013-10-31 at 15:15 +0100, Jesper Dangaard Brouer wrote:
>
> > Okay, I'll cook up another patch, after work.
> >
> > Adding all the typecheck() stuff, just bloats the code.
> >
> > Would it be better/okay just to do?:
> > (s32)((u32)(a) - (u32)(b)) > 0)
> >
> >
>
> What about using the existing codel types ?
Hmm, I would be okay to use codel types for typecheck(), but I don't
like the approach below, because we are hiding a typecast. This just
makes the code harder to read/understand. An explicit cast shows that
we are doing something nasty, on purpose here.
I would rather keep as close as possible to include/linux/jiffies.h,
because I want readers to be-able to spot this pattern.
> diff --git a/include/net/codel.h b/include/net/codel.h
> index 389cf62..89a7781 100644
> --- a/include/net/codel.h
> +++ b/include/net/codel.h
> @@ -72,7 +72,12 @@ static inline codel_time_t codel_get_time(void)
> return ns >> CODEL_SHIFT;
> }
>
> -#define codel_time_after(a, b) ((s32)(a) - (s32)(b) > 0)
> +static inline bool codel_time_after(codel_time_t a, codel_time_t b)
> +{
> + codel_tdiff_t delta = a - b;
> +
> + return delta >= 0;
> +}
> #define codel_time_after_eq(a, b) ((s32)(a) - (s32)(b) >= 0)
> #define codel_time_before(a, b) ((s32)(a) - (s32)(b) < 0)
> #define codel_time_before_eq(a, b) ((s32)(a) - (s32)(b) <= 0)
>
>
> You need of course something similar for all variants.
>
>
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [net-next PATCH V2] net: codel: Avoid undefined behavior from signed overflow
From: Jesper Dangaard Brouer @ 2013-10-31 21:10 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet, Paul E. McKenney, Dave Taht, Jesper Dangaard Brouer
From: Jesper Dangaard Brouer <netoptimizer@brouer.com>
As described in commit 5a581b367 (jiffies: Avoid undefined
behavior from signed overflow), according to the C standard
3.4.3p3, overflow of a signed integer results in undefined
behavior.
To fix this, do as the above commit, and do an unsigned
subtraction, and interpreting the result as a signed
two's-complement number. This is based on the theory from
RFC 1982 and is nicely described in wikipedia here:
https://en.wikipedia.org/wiki/Serial_number_arithmetic#General_Solution
A side-note, I have seen practical issues with the previous logic
when dealing with 16-bit, on a 64-bit machine (gcc version
4.4.5). This were 32-bit, which I have not observed issues with.
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Jesper Dangaard Brouer <netoptimizer@brouer.com>
---
include/net/codel.h | 19 +++++++++++++++----
1 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/include/net/codel.h b/include/net/codel.h
index 389cf62..3b04ff5 100644
--- a/include/net/codel.h
+++ b/include/net/codel.h
@@ -72,10 +72,21 @@ static inline codel_time_t codel_get_time(void)
return ns >> CODEL_SHIFT;
}
-#define codel_time_after(a, b) ((s32)(a) - (s32)(b) > 0)
-#define codel_time_after_eq(a, b) ((s32)(a) - (s32)(b) >= 0)
-#define codel_time_before(a, b) ((s32)(a) - (s32)(b) < 0)
-#define codel_time_before_eq(a, b) ((s32)(a) - (s32)(b) <= 0)
+/* Dealing with timer wrapping, according to RFC 1982, as desc in wikipedia:
+ * https://en.wikipedia.org/wiki/Serial_number_arithmetic#General_Solution
+ * codel_time_after(a,b) returns true if the time a is after time b.
+ */
+#define codel_time_after(a, b) \
+ (typecheck(codel_time_t, a) && \
+ typecheck(codel_time_t, b) && \
+ ((s32)((a) - (b)) > 0))
+#define codel_time_before(a, b) codel_time_after(b, a)
+
+#define codel_time_after_eq(a, b) \
+ (typecheck(codel_time_t, a) && \
+ typecheck(codel_time_t, b) && \
+ ((s32)((a) - (b)) >= 0))
+#define codel_time_before_eq(a, b) codel_time_after_eq(b, a)
/* Qdiscs using codel plugin must use codel_skb_cb in their own cb[] */
struct codel_skb_cb {
^ permalink raw reply related
* Re: [net-next PATCH] net: codel: Avoid undefined behavior from signed overflow
From: Jesper Dangaard Brouer @ 2013-10-31 21:53 UTC (permalink / raw)
To: Ben Hutchings
Cc: Jesper Dangaard Brouer, netdev, Eric Dumazet, Paul E. McKenney,
Dave Taht, Eilon Greenstein
In-Reply-To: <1383161748.1601.24.camel@bwh-desktop.uk.level5networks.com>
On Wed, 30 Oct 2013 19:35:48 +0000
Ben Hutchings <bhutchings@solarflare.com> wrote:
> On Wed, 2013-10-30 at 18:23 +0100, Jesper Dangaard Brouer wrote:
> > From: Jesper Dangaard Brouer <netoptimizer@brouer.com>
> >
> > As described in commit 5a581b367 (jiffies: Avoid undefined
> > behavior from signed overflow), according to the C standard
> > 3.4.3p3, overflow of a signed integer results in undefined
> > behavior.
> [...]
>
> According to the real processors that Linux runs on, signed arithmetic
> uses 2's complement representation and overflow wraps accordingly. And
> we rely on that behaviour in many places, so we use
> '-fno-strict-overflow' to tell gcc not to assume we avoid signed
> overflow. (There is also '-fwrapv' which tells gcc to assume the
> processor behaves this way, but shouldn't it already know how the target
> machine works?)
For 16-bit I have tested that is fails, and that it does not help to
use the compiler flag: '-fno-strict-overflow' or '-fwrapv'. (this was
userspace test code, so I might be missing some kernel compiler options
that would make this work for 16-bit, but I doubt it)
#define works_u16_time_after(a,b) \
(typecheck(u_int16_t, a) && \
typecheck(u_int16_t, b) && \
((int16_t)((b) - (a)) < 0))
#define bad_u16_time_after(a,b) \
(typecheck(u_int16_t, a) && \
typecheck(u_int16_t, b) && \
(((int16_t)(b) - (int16_t)(a)) < 0))
The bnx2x have a wrong/dangerup construct:
File: drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
#define SUB_S16(a, b) (s16)((s16)(a) - (s16)(b))
#define SUB_S32(a, b) (s32)((s32)(a) - (s32)(b))
I have tested this case, and it surprisingly works, due to the outer
(s16) cast I believe.
I think this should/could be fixed like:
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
index 4e01c57..8969733 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
@@ -838,8 +838,8 @@ static inline bool bnx2x_fp_ll_polling(struct bnx2x_fastpath *fp)
#define RCQ_TH_HI(bp) (RCQ_TH_LO(bp) + DROPLESS_FC_HEADROOM)
/* This is needed for determining of last_max */
-#define SUB_S16(a, b) (s16)((s16)(a) - (s16)(b))
-#define SUB_S32(a, b) (s32)((s32)(a) - (s32)(b))
+#define SUB_S16(a, b) (s16)((u16)(a) - (u16)(b))
+#define SUB_S32(a, b) (s32)((u32)(a) - (u32)(b))
#define BNX2X_SWCID_SHIFT 17
#define BNX2X_SWCID_MASK ((0x1 << BNX2X_SWCID_SHIFT) - 1)
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply related
* Re: [PATCH net-next] tcp: enable sockets to use MSG_FASTOPEN by default
From: Eric Dumazet @ 2013-10-31 23:19 UTC (permalink / raw)
To: Yuchung Cheng; +Cc: davem, edumazet, netdev, ncardwell, sivasankar
In-Reply-To: <1383236372-1317-1-git-send-email-ycheng@google.com>
On Thu, 2013-10-31 at 09:19 -0700, Yuchung Cheng wrote:
> Applications have started to use Fast Open (e.g., Chrome browser has
> such an optional flag) and the feature has gone through several
> generations of kernels since 3.7 with many real network tests. It's
> time to enable this flag by default for applications to test more
> conveniently and extensively.
>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> ---
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* [PATCH NEXT V2] rtlwifi: Fix endian error in extracting packet type
From: Larry Finger @ 2013-11-01 0:43 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Mark Cave-Ayland, netdev, Larry Finger, Stable
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
All of the rtlwifi drivers have an error in the routine that tests if
the data is "special". If it is, the subsequant transmission will be
at the lowest rate to enhance reliability. The 16-bit quantity is
big-endian, but was being extracted in native CPU mode. One of the
effects of this bug is to inhibit association under some conditions
as the TX rate is too high.
A statement that would have made the code correct had been changed to
a comment. Rather than just reinstating that code, the fix here passes
sparse tests. A side effect of fixing this problem would have been to force
all IPv6 frames to run at the lowest rate. The test for that frame type
is removed.
The original code only checked the lower-order byte of UDP ports for BOOTP
protocol. That is extended to the full 16-bit source and destination ports.
One of the local headers contained duplicates of some of the ETH_P_XXX
definitions. These are deleted.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Stable <stable@vger.kernel.org> [2.6.38+]
---
V2 - Addresses comments by Ben Hutchings and Bjorn Mork.
drivers/net/wireless/rtlwifi/base.c | 15 ++++++---------
drivers/net/wireless/rtlwifi/wifi.h | 6 +-----
2 files changed, 7 insertions(+), 14 deletions(-)
Index: wireless-testing-save/drivers/net/wireless/rtlwifi/base.c
===================================================================
--- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/base.c
+++ wireless-testing-save/drivers/net/wireless/rtlwifi/base.c
@@ -1077,17 +1077,17 @@ u8 rtl_is_special_data(struct ieee80211_
ip = (struct iphdr *)((u8 *) skb->data + mac_hdr_len +
SNAP_SIZE + PROTOC_TYPE_SIZE);
- ether_type = *(u16 *) ((u8 *) skb->data + mac_hdr_len + SNAP_SIZE);
- /* ether_type = ntohs(ether_type); */
+ ether_type = be16_to_cpu(*(__be16 *)((u8 *)skb->data + mac_hdr_len +
+ SNAP_SIZE));
if (ETH_P_IP == ether_type) {
if (IPPROTO_UDP == ip->protocol) {
struct udphdr *udp = (struct udphdr *)((u8 *) ip +
(ip->ihl << 2));
- if (((((u8 *) udp)[1] == 68) &&
- (((u8 *) udp)[3] == 67)) ||
- ((((u8 *) udp)[1] == 67) &&
- (((u8 *) udp)[3] == 68))) {
+ if (((((u16 *) udp)[0] == 68) &&
+ (((u16 *) udp)[2] == 67)) ||
+ ((((u16 *) udp)[0] == 67) &&
+ (((u16 *) udp)[2] == 68))) {
/*
* 68 : UDP BOOTP client
* 67 : UDP BOOTP server
@@ -1126,9 +1126,6 @@ u8 rtl_is_special_data(struct ieee80211_
}
return true;
- } else if (ETH_P_IPV6 == ether_type) {
- /* IPv6 */
- return true;
}
return false;
Index: wireless-testing-save/drivers/net/wireless/rtlwifi/wifi.h
===================================================================
--- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/wifi.h
+++ wireless-testing-save/drivers/net/wireless/rtlwifi/wifi.h
@@ -77,11 +77,7 @@
#define RTL_SLOT_TIME_9 9
#define RTL_SLOT_TIME_20 20
-/*related with tcp/ip. */
-/*if_ehther.h*/
-#define ETH_P_PAE 0x888E /*Port Access Entity (IEEE 802.1X) */
-#define ETH_P_IP 0x0800 /*Internet Protocol packet */
-#define ETH_P_ARP 0x0806 /*Address Resolution packet */
+/*related to tcp/ip. */
#define SNAP_SIZE 6
#define PROTOC_TYPE_SIZE 2
^ permalink raw reply
* Re: [PATCH NEXT V2] rtlwifi: Fix endian error in extracting packet type
From: Ben Hutchings @ 2013-11-01 1:02 UTC (permalink / raw)
To: Larry Finger
Cc: linville-2XuSBdqkA4R54TAoqtyWWQ,
linux-wireless-u79uwXL29TY76Z2rM5mHXA, Mark Cave-Ayland,
netdev-u79uwXL29TY76Z2rM5mHXA, Stable
In-Reply-To: <1383266638-4813-1-git-send-email-Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
On Thu, 2013-10-31 at 19:43 -0500, Larry Finger wrote:
> From: Mark Cave-Ayland <mark.cave-ayland-r2/yp8inhXLQXOPxS62xeg@public.gmane.org>
>
> All of the rtlwifi drivers have an error in the routine that tests if
> the data is "special". If it is, the subsequant transmission will be
> at the lowest rate to enhance reliability. The 16-bit quantity is
> big-endian, but was being extracted in native CPU mode. One of the
> effects of this bug is to inhibit association under some conditions
> as the TX rate is too high.
>
> A statement that would have made the code correct had been changed to
> a comment. Rather than just reinstating that code, the fix here passes
> sparse tests. A side effect of fixing this problem would have been to force
> all IPv6 frames to run at the lowest rate. The test for that frame type
> is removed.
>
> The original code only checked the lower-order byte of UDP ports for BOOTP
> protocol. That is extended to the full 16-bit source and destination ports.
>
> One of the local headers contained duplicates of some of the ETH_P_XXX
> definitions. These are deleted.
>
> Signed-off-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
> Cc: Mark Cave-Ayland <mark.cave-ayland-r2/yp8inhXLQXOPxS62xeg@public.gmane.org>
> Cc: Stable <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> [2.6.38+]
> ---
>
> V2 - Addresses comments by Ben Hutchings and Bjorn Mork.
>
> drivers/net/wireless/rtlwifi/base.c | 15 ++++++---------
> drivers/net/wireless/rtlwifi/wifi.h | 6 +-----
> 2 files changed, 7 insertions(+), 14 deletions(-)
> Index: wireless-testing-save/drivers/net/wireless/rtlwifi/base.c
> ===================================================================
> --- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/base.c
> +++ wireless-testing-save/drivers/net/wireless/rtlwifi/base.c
> @@ -1077,17 +1077,17 @@ u8 rtl_is_special_data(struct ieee80211_
>
> ip = (struct iphdr *)((u8 *) skb->data + mac_hdr_len +
> SNAP_SIZE + PROTOC_TYPE_SIZE);
> - ether_type = *(u16 *) ((u8 *) skb->data + mac_hdr_len + SNAP_SIZE);
> - /* ether_type = ntohs(ether_type); */
> + ether_type = be16_to_cpu(*(__be16 *)((u8 *)skb->data + mac_hdr_len +
> + SNAP_SIZE));
>
> if (ETH_P_IP == ether_type) {
> if (IPPROTO_UDP == ip->protocol) {
> struct udphdr *udp = (struct udphdr *)((u8 *) ip +
> (ip->ihl << 2));
> - if (((((u8 *) udp)[1] == 68) &&
> - (((u8 *) udp)[3] == 67)) ||
> - ((((u8 *) udp)[1] == 67) &&
> - (((u8 *) udp)[3] == 68))) {
> + if (((((u16 *) udp)[0] == 68) &&
> + (((u16 *) udp)[2] == 67)) ||
> + ((((u16 *) udp)[0] == 67) &&
> + (((u16 *) udp)[2] == 68))) {
[...]
Now you're missing byte-swapping here, and using the wrong offset for
the dest port (4 bytes rather than 2).
If you really think this is necessary then use something like:
if ((udp->source == htons(68) &&
udp->dest == htons(67)) ||
...
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [Linuxptp-devel] RFC [PATCH 2/3] PTP: use flags to request HW features
From: Flavio Leitner @ 2013-11-01 1:34 UTC (permalink / raw)
To: Richard Cochran; +Cc: linuxptp-devel, netdev, Ben Hutchings
In-Reply-To: <20131031071256.GB5226@netboy>
On Thu, Oct 31, 2013 at 08:12:58AM +0100, Richard Cochran wrote:
> On Wed, Oct 30, 2013 at 07:48:41PM -0200, Flavio Leitner wrote:
> > On Wed, Oct 30, 2013 at 05:00:36PM -0200, Flavio Leitner wrote:
> > > Currently the user space can't tell which delay mechanism
> > > (E2E/P2P) or transport is needed in the ioctl(). Therefore,
> > > PTP silently fails when the hardware doesn't support a
> > > certain delay mechanism or transport.
>
> [ For netdev readers, the text below repeats what I posted to the
> linuxptp-devel list. ]
I am reconsidering about extending ethtool instead, so I'd like to
drop these patches for now.
Thanks Richard and Jacob for the contributions,
fbl
^ permalink raw reply
* Re: [PATCH NEXT V2] rtlwifi: Fix endian error in extracting packet type
From: Joe Perches @ 2013-11-01 1:48 UTC (permalink / raw)
To: Ben Hutchings
Cc: Larry Finger, linville, linux-wireless, Mark Cave-Ayland, netdev,
Stable
In-Reply-To: <1383267769.1583.8.camel@bwh-desktop.uk.level5networks.com>
On Fri, 2013-11-01 at 01:02 +0000, Ben Hutchings wrote:
> On Thu, 2013-10-31 at 19:43 -0500, Larry Finger wrote:
> > From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> >
> > All of the rtlwifi drivers have an error in the routine that tests if
> > the data is "special". If it is, the subsequant transmission will be
> > at the lowest rate to enhance reliability. The 16-bit quantity is
> > big-endian, but was being extracted in native CPU mode. One of the
> > effects of this bug is to inhibit association under some conditions
> > as the TX rate is too high.
> >
> > A statement that would have made the code correct had been changed to
> > a comment. Rather than just reinstating that code, the fix here passes
> > sparse tests. A side effect of fixing this problem would have been to force
> > all IPv6 frames to run at the lowest rate. The test for that frame type
> > is removed.
> >
> > The original code only checked the lower-order byte of UDP ports for BOOTP
> > protocol. That is extended to the full 16-bit source and destination ports.
> >
> > One of the local headers contained duplicates of some of the ETH_P_XXX
> > definitions. These are deleted.
> >
> > Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> > Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > Cc: Stable <stable@vger.kernel.org> [2.6.38+]
> > ---
> >
> > V2 - Addresses comments by Ben Hutchings and Bjorn Mork.
> >
> > drivers/net/wireless/rtlwifi/base.c | 15 ++++++---------
> > drivers/net/wireless/rtlwifi/wifi.h | 6 +-----
> > 2 files changed, 7 insertions(+), 14 deletions(-)
> > Index: wireless-testing-save/drivers/net/wireless/rtlwifi/base.c
> > ===================================================================
> > --- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/base.c
> > +++ wireless-testing-save/drivers/net/wireless/rtlwifi/base.c
> > @@ -1077,17 +1077,17 @@ u8 rtl_is_special_data(struct ieee80211_
> >
> > ip = (struct iphdr *)((u8 *) skb->data + mac_hdr_len +
> > SNAP_SIZE + PROTOC_TYPE_SIZE);
> > - ether_type = *(u16 *) ((u8 *) skb->data + mac_hdr_len + SNAP_SIZE);
> > - /* ether_type = ntohs(ether_type); */
> > + ether_type = be16_to_cpu(*(__be16 *)((u8 *)skb->data + mac_hdr_len +
> > + SNAP_SIZE));
> >
> > if (ETH_P_IP == ether_type) {
> > if (IPPROTO_UDP == ip->protocol) {
> > struct udphdr *udp = (struct udphdr *)((u8 *) ip +
> > (ip->ihl << 2));
> > - if (((((u8 *) udp)[1] == 68) &&
> > - (((u8 *) udp)[3] == 67)) ||
> > - ((((u8 *) udp)[1] == 67) &&
> > - (((u8 *) udp)[3] == 68))) {
> > + if (((((u16 *) udp)[0] == 68) &&
> > + (((u16 *) udp)[2] == 67)) ||
> > + ((((u16 *) udp)[0] == 67) &&
> > + (((u16 *) udp)[2] == 68))) {
> [...]
>
> Now you're missing byte-swapping here, and using the wrong offset for
> the dest port (4 bytes rather than 2).
>
> If you really think this is necessary then use something like:
> if ((udp->source == htons(68) &&
> udp->dest == htons(67)) ||
> ...
Or maybe something like this?
---
drivers/net/wireless/rtlwifi/base.c | 91 +++++++++++++++++--------------------
1 file changed, 41 insertions(+), 50 deletions(-)
diff --git a/drivers/net/wireless/rtlwifi/base.c b/drivers/net/wireless/rtlwifi/base.c
index 9a78e3d..7e9df65 100644
--- a/drivers/net/wireless/rtlwifi/base.c
+++ b/drivers/net/wireless/rtlwifi/base.c
@@ -37,6 +37,7 @@
#include <linux/ip.h>
#include <linux/module.h>
+#include <linux/udp.h>
/*
*NOTICE!!!: This file will be very big, we should
@@ -1074,64 +1075,54 @@ u8 rtl_is_special_data(struct ieee80211_hw *hw, struct sk_buff *skb, u8 is_tx)
if (!ieee80211_is_data(fc))
return false;
+ ip = (const struct iphdr *)(skb->data + mac_hdr_len +
+ SNAP_SIZE + PROTOC_TYPE_SIZE);
+ ether_type = be16_to_cpup((__be16 *)
+ (skb->data + mac_hdr_len + SNAP_SIZE));
- ip = (struct iphdr *)((u8 *) skb->data + mac_hdr_len +
- SNAP_SIZE + PROTOC_TYPE_SIZE);
- ether_type = *(u16 *) ((u8 *) skb->data + mac_hdr_len + SNAP_SIZE);
- /* ether_type = ntohs(ether_type); */
-
- if (ETH_P_IP == ether_type) {
- if (IPPROTO_UDP == ip->protocol) {
- struct udphdr *udp = (struct udphdr *)((u8 *) ip +
- (ip->ihl << 2));
- if (((((u8 *) udp)[1] == 68) &&
- (((u8 *) udp)[3] == 67)) ||
- ((((u8 *) udp)[1] == 67) &&
- (((u8 *) udp)[3] == 68))) {
- /*
- * 68 : UDP BOOTP client
- * 67 : UDP BOOTP server
- */
- RT_TRACE(rtlpriv, (COMP_SEND | COMP_RECV),
- DBG_DMESG, "dhcp %s !!\n",
- is_tx ? "Tx" : "Rx");
-
- if (is_tx) {
- rtlpriv->enter_ps = false;
- schedule_work(&rtlpriv->
- works.lps_change_work);
- ppsc->last_delaylps_stamp_jiffies =
- jiffies;
- }
+ switch (ether_type) {
+ case ETH_P_IP: {
+ struct udphdr *udp;
+ u16 src;
+ u16 dst;
- return true;
- }
- }
- } else if (ETH_P_ARP == ether_type) {
- if (is_tx) {
- rtlpriv->enter_ps = false;
- schedule_work(&rtlpriv->works.lps_change_work);
- ppsc->last_delaylps_stamp_jiffies = jiffies;
- }
+ if (ip->protocol != IPPROTO_UDP)
+ return false;
- return true;
- } else if (ETH_P_PAE == ether_type) {
- RT_TRACE(rtlpriv, (COMP_SEND | COMP_RECV), DBG_DMESG,
- "802.1X %s EAPOL pkt!!\n", is_tx ? "Tx" : "Rx");
+ udp = (struct udphdr *)((u8 *)ip + (ip->ihl << 2));
+ src = be16_to_cpu(udp->source) >> 8;
+ dst = be16_to_cpu(udp->dest) >> 8;
- if (is_tx) {
- rtlpriv->enter_ps = false;
- schedule_work(&rtlpriv->works.lps_change_work);
- ppsc->last_delaylps_stamp_jiffies = jiffies;
- }
+ /*
+ * 68 : UDP BOOTP client
+ * 67 : UDP BOOTP server
+ */
+ if (!((src == 68 && dst == 67) || (src == 67 && dst == 68)))
+ return false;
+ RT_TRACE(rtlpriv, (COMP_SEND | COMP_RECV), DBG_DMESG,
+ "dhcp %s !!\n", is_tx ? "Tx" : "Rx");
+ break;
+ }
+ case ETH_P_ARP:
+ break;
+ case ETH_P_PAE:
+ RT_TRACE(rtlpriv, (COMP_SEND | COMP_RECV), DBG_DMESG,
+ "802.1X %s EAPOL pkt!!\n", is_tx ? "Tx" : "Rx");
+ break;
+ case ETH_P_IPV6:
return true;
- } else if (ETH_P_IPV6 == ether_type) {
- /* IPv6 */
- return true;
+ default:
+ return false;
}
- return false;
+ if (is_tx) {
+ rtlpriv->enter_ps = false;
+ schedule_work(&rtlpriv->works.lps_change_work);
+ ppsc->last_delaylps_stamp_jiffies = jiffies;
+ }
+
+ return true;
}
EXPORT_SYMBOL_GPL(rtl_is_special_data);
^ permalink raw reply related
* Re: [PATCH NEXT V2] rtlwifi: Fix endian error in extracting packet type
From: Joe Perches @ 2013-11-01 2:04 UTC (permalink / raw)
To: Ben Hutchings
Cc: Larry Finger, linville, linux-wireless, Mark Cave-Ayland, netdev,
Stable
In-Reply-To: <1383270515.2769.15.camel@joe-AO722>
On Thu, 2013-10-31 at 18:48 -0700, Joe Perches wrote:
> Or maybe something like this?
[]
> diff --git a/drivers/net/wireless/rtlwifi/base.c b/drivers/net/wireless/rtlwifi/base.c
[]
> + src = be16_to_cpu(udp->source) >> 8;
> + dst = be16_to_cpu(udp->dest) >> 8;
Of course this doesn't need the >> 8;
^ 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