* Re: [PATCH net-next v3 1/2] bpf: add support for sys_enter_* and sys_exit_* tracepoints
From: Alexei Starovoitov @ 2017-08-04 2:08 UTC (permalink / raw)
To: Yonghong Song, peterz, rostedt, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170803162951.1564963-2-yhs@fb.com>
On 8/3/17 6:29 AM, Yonghong Song wrote:
> @@ -578,8 +596,9 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
> if (!sys_data)
> return;
>
> + prog = READ_ONCE(sys_data->enter_event->prog);
> head = this_cpu_ptr(sys_data->enter_event->perf_events);
> - if (hlist_empty(head))
> + if (!prog && hlist_empty(head))
> return;
>
> /* get the size after alignment with the u32 buffer size field */
> @@ -594,6 +613,13 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
> rec->nr = syscall_nr;
> syscall_get_arguments(current, regs, 0, sys_data->nb_args,
> (unsigned long *)&rec->args);
> +
> + if ((prog && !perf_call_bpf_enter(prog, regs, sys_data, rec)) ||
> + hlist_empty(head)) {
> + perf_swevent_put_recursion_context(rctx);
> + return;
> + }
hmm. if I read the patch correctly that makes it different from
kprobe/uprobe/tracepoints+bpf behavior. Why make it different and
force user space to perf_event_open() on every cpu?
In other cases it's the job of the bpf program to filter by cpu
if necessary and that is well understood by bcc scripts.
^ permalink raw reply
* Re: Gift-
From: Mayrhofer Family @ 2017-08-04 1:18 UTC (permalink / raw)
To: Recipients
Good Day,
My wife and I have awarded you with a donation of $ 1,000,000.00 Dollars from part of our Jackpot Lottery of 50 Million Dollars, respond with your details for claims.
We await your earliest response and God Bless you.
Friedrich And Annand Mayrhofer.
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
^ permalink raw reply
* Re: [PATCH net-next v3 1/2] bpf: add support for sys_enter_* and sys_exit_* tracepoints
From: Y Song @ 2017-08-04 3:09 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Yonghong Song, peterz, rostedt, Daniel Borkmann, netdev,
kernel-team
In-Reply-To: <11ef74f9-b0ef-264f-f582-62a8c8f1f027@fb.com>
On Thu, Aug 3, 2017 at 7:08 PM, Alexei Starovoitov <ast@fb.com> wrote:
> On 8/3/17 6:29 AM, Yonghong Song wrote:
>>
>> @@ -578,8 +596,9 @@ static void perf_syscall_enter(void *ignore, struct
>> pt_regs *regs, long id)
>> if (!sys_data)
>> return;
>>
>> + prog = READ_ONCE(sys_data->enter_event->prog);
>> head = this_cpu_ptr(sys_data->enter_event->perf_events);
>> - if (hlist_empty(head))
>> + if (!prog && hlist_empty(head))
>> return;
>>
>> /* get the size after alignment with the u32 buffer size field */
>> @@ -594,6 +613,13 @@ static void perf_syscall_enter(void *ignore, struct
>> pt_regs *regs, long id)
>> rec->nr = syscall_nr;
>> syscall_get_arguments(current, regs, 0, sys_data->nb_args,
>> (unsigned long *)&rec->args);
>> +
>> + if ((prog && !perf_call_bpf_enter(prog, regs, sys_data, rec)) ||
>> + hlist_empty(head)) {
>> + perf_swevent_put_recursion_context(rctx);
>> + return;
>> + }
>
>
> hmm. if I read the patch correctly that makes it different from
> kprobe/uprobe/tracepoints+bpf behavior. Why make it different and
> force user space to perf_event_open() on every cpu?
> In other cases it's the job of the bpf program to filter by cpu
> if necessary and that is well understood by bcc scripts.
The patch actually does allow the bpf program to track all cpus.
The test:
>> + if (!prog && hlist_empty(head))
>> return;
ensures that if prog is not empty, it will not return even if the
event in the current cpu is empty. Later on, perf_call_bpf_enter will
be called if prog is not empty. This ensures that
the bpf program will execute regardless of the current cpu.
Maybe I missed anything here?
^ permalink raw reply
* [PATCH 0/2 v2 net-next] tcp cwnd undo refactor
From: Yuchung Cheng @ 2017-08-04 3:38 UTC (permalink / raw)
To: davem; +Cc: netdev, ncardwell, unlcsewsun, stephen, Yuchung Cheng
This patch series consolidate similar cwnd undo functions
implemented by various congestion control by using existing
tcp socket state variable. The first patch fixes a corner
case in of cwnd undo in Reno and HTCP. Since the bug has
existed for many years and is very minor, we consider this
patch set more suitable for net-next as the major change
is the refactor itself.
- v1->v2
Fix trivial compile errors
Yuchung Cheng (2):
tcp: fix cwnd undo in Reno and HTCP congestion controls
tcp: consolidate congestion control undo functions
include/linux/tcp.h | 2 +-
net/ipv4/tcp_bic.c | 14 +-------------
net/ipv4/tcp_cdg.c | 12 +-----------
net/ipv4/tcp_cong.c | 2 +-
net/ipv4/tcp_cubic.c | 13 +------------
net/ipv4/tcp_highspeed.c | 11 +----------
net/ipv4/tcp_htcp.c | 3 +--
net/ipv4/tcp_illinois.c | 11 +----------
net/ipv4/tcp_input.c | 1 +
net/ipv4/tcp_nv.c | 13 +------------
net/ipv4/tcp_scalable.c | 16 +---------------
net/ipv4/tcp_veno.c | 11 +----------
net/ipv4/tcp_yeah.c | 11 +----------
13 files changed, 13 insertions(+), 107 deletions(-)
--
2.14.0.rc1.383.gd1ce394fe2-goog
^ permalink raw reply
* [PATCH 1/2 v2 net-next] tcp: fix cwnd undo in Reno and HTCP congestion controls
From: Yuchung Cheng @ 2017-08-04 3:38 UTC (permalink / raw)
To: davem; +Cc: netdev, ncardwell, unlcsewsun, stephen, Yuchung Cheng
In-Reply-To: <20170804033852.97986-1-ycheng@google.com>
Using ssthresh to revert cwnd is less reliable when ssthresh is
bounded to 2 packets. This patch uses an existing variable in TCP
"prior_cwnd" that snapshots the cwnd right before entering fast
recovery and RTO recovery in Reno. This fixes the issue discussed
in netdev thread: "A buggy behavior for Linux TCP Reno and HTCP"
https://www.spinics.net/lists/netdev/msg444955.html
Suggested-by: Neal Cardwell <ncardwell@google.com>
Reported-by: Wei Sun <unlcsewsun@gmail.com>
Signed-off-by: Yuchung Cheng <ncardwell@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
include/linux/tcp.h | 2 +-
net/ipv4/tcp_cong.c | 2 +-
net/ipv4/tcp_htcp.c | 3 +--
net/ipv4/tcp_input.c | 1 +
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index d7389ea36e10..267164a1d559 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -258,7 +258,7 @@ struct tcp_sock {
u32 snd_cwnd_clamp; /* Do not allow snd_cwnd to grow above this */
u32 snd_cwnd_used;
u32 snd_cwnd_stamp;
- u32 prior_cwnd; /* Congestion window at start of Recovery. */
+ u32 prior_cwnd; /* cwnd right before starting loss recovery */
u32 prr_delivered; /* Number of newly delivered packets to
* receiver in Recovery. */
u32 prr_out; /* Total number of pkts sent during Recovery. */
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
index fde983f6376b..c2b174469645 100644
--- a/net/ipv4/tcp_cong.c
+++ b/net/ipv4/tcp_cong.c
@@ -456,7 +456,7 @@ u32 tcp_reno_undo_cwnd(struct sock *sk)
{
const struct tcp_sock *tp = tcp_sk(sk);
- return max(tp->snd_cwnd, tp->snd_ssthresh << 1);
+ return max(tp->snd_cwnd, tp->prior_cwnd);
}
EXPORT_SYMBOL_GPL(tcp_reno_undo_cwnd);
diff --git a/net/ipv4/tcp_htcp.c b/net/ipv4/tcp_htcp.c
index 3eb78cde6ff0..082d479462fa 100644
--- a/net/ipv4/tcp_htcp.c
+++ b/net/ipv4/tcp_htcp.c
@@ -66,7 +66,6 @@ static inline void htcp_reset(struct htcp *ca)
static u32 htcp_cwnd_undo(struct sock *sk)
{
- const struct tcp_sock *tp = tcp_sk(sk);
struct htcp *ca = inet_csk_ca(sk);
if (ca->undo_last_cong) {
@@ -76,7 +75,7 @@ static u32 htcp_cwnd_undo(struct sock *sk)
ca->undo_last_cong = 0;
}
- return max(tp->snd_cwnd, (tp->snd_ssthresh << 7) / ca->beta);
+ return tcp_reno_undo_cwnd(sk);
}
static inline void measure_rtt(struct sock *sk, u32 srtt)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 99cdf4ccabb8..842ed75ccb25 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1950,6 +1950,7 @@ void tcp_enter_loss(struct sock *sk)
!after(tp->high_seq, tp->snd_una) ||
(icsk->icsk_ca_state == TCP_CA_Loss && !icsk->icsk_retransmits)) {
tp->prior_ssthresh = tcp_current_ssthresh(sk);
+ tp->prior_cwnd = tp->snd_cwnd;
tp->snd_ssthresh = icsk->icsk_ca_ops->ssthresh(sk);
tcp_ca_event(sk, CA_EVENT_LOSS);
tcp_init_undo(tp);
--
2.14.0.rc1.383.gd1ce394fe2-goog
^ permalink raw reply related
* [PATCH 2/2 v2 net-next] tcp: consolidate congestion control undo functions
From: Yuchung Cheng @ 2017-08-04 3:38 UTC (permalink / raw)
To: davem; +Cc: netdev, ncardwell, unlcsewsun, stephen, Yuchung Cheng
In-Reply-To: <20170804033852.97986-1-ycheng@google.com>
Most TCP congestion controls are using identical logic to undo
cwnd except BBR. This patch consolidates these similar functions
to the one used currently by Reno and others.
Suggested-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
net/ipv4/tcp_bic.c | 14 +-------------
net/ipv4/tcp_cdg.c | 12 +-----------
net/ipv4/tcp_cubic.c | 13 +------------
net/ipv4/tcp_highspeed.c | 11 +----------
net/ipv4/tcp_illinois.c | 11 +----------
net/ipv4/tcp_nv.c | 13 +------------
net/ipv4/tcp_scalable.c | 16 +---------------
net/ipv4/tcp_veno.c | 11 +----------
net/ipv4/tcp_yeah.c | 11 +----------
9 files changed, 9 insertions(+), 103 deletions(-)
diff --git a/net/ipv4/tcp_bic.c b/net/ipv4/tcp_bic.c
index 609965f0e298..fc3614377413 100644
--- a/net/ipv4/tcp_bic.c
+++ b/net/ipv4/tcp_bic.c
@@ -49,7 +49,6 @@ MODULE_PARM_DESC(smooth_part, "log(B/(B*Smin))/log(B/(B-1))+B, # of RTT from Wma
struct bictcp {
u32 cnt; /* increase cwnd by 1 after ACKs */
u32 last_max_cwnd; /* last maximum snd_cwnd */
- u32 loss_cwnd; /* congestion window at last loss */
u32 last_cwnd; /* the last snd_cwnd */
u32 last_time; /* time when updated last_cwnd */
u32 epoch_start; /* beginning of an epoch */
@@ -72,7 +71,6 @@ static void bictcp_init(struct sock *sk)
struct bictcp *ca = inet_csk_ca(sk);
bictcp_reset(ca);
- ca->loss_cwnd = 0;
if (initial_ssthresh)
tcp_sk(sk)->snd_ssthresh = initial_ssthresh;
@@ -172,22 +170,12 @@ static u32 bictcp_recalc_ssthresh(struct sock *sk)
else
ca->last_max_cwnd = tp->snd_cwnd;
- ca->loss_cwnd = tp->snd_cwnd;
-
if (tp->snd_cwnd <= low_window)
return max(tp->snd_cwnd >> 1U, 2U);
else
return max((tp->snd_cwnd * beta) / BICTCP_BETA_SCALE, 2U);
}
-static u32 bictcp_undo_cwnd(struct sock *sk)
-{
- const struct tcp_sock *tp = tcp_sk(sk);
- const struct bictcp *ca = inet_csk_ca(sk);
-
- return max(tp->snd_cwnd, ca->loss_cwnd);
-}
-
static void bictcp_state(struct sock *sk, u8 new_state)
{
if (new_state == TCP_CA_Loss)
@@ -214,7 +202,7 @@ static struct tcp_congestion_ops bictcp __read_mostly = {
.ssthresh = bictcp_recalc_ssthresh,
.cong_avoid = bictcp_cong_avoid,
.set_state = bictcp_state,
- .undo_cwnd = bictcp_undo_cwnd,
+ .undo_cwnd = tcp_reno_undo_cwnd,
.pkts_acked = bictcp_acked,
.owner = THIS_MODULE,
.name = "bic",
diff --git a/net/ipv4/tcp_cdg.c b/net/ipv4/tcp_cdg.c
index 50a0f3e51d5b..66ac69f7bd19 100644
--- a/net/ipv4/tcp_cdg.c
+++ b/net/ipv4/tcp_cdg.c
@@ -85,7 +85,6 @@ struct cdg {
u8 state;
u8 delack;
u32 rtt_seq;
- u32 undo_cwnd;
u32 shadow_wnd;
u16 backoff_cnt;
u16 sample_cnt;
@@ -330,8 +329,6 @@ static u32 tcp_cdg_ssthresh(struct sock *sk)
struct cdg *ca = inet_csk_ca(sk);
struct tcp_sock *tp = tcp_sk(sk);
- ca->undo_cwnd = tp->snd_cwnd;
-
if (ca->state == CDG_BACKOFF)
return max(2U, (tp->snd_cwnd * min(1024U, backoff_beta)) >> 10);
@@ -344,13 +341,6 @@ static u32 tcp_cdg_ssthresh(struct sock *sk)
return max(2U, tp->snd_cwnd >> 1);
}
-static u32 tcp_cdg_undo_cwnd(struct sock *sk)
-{
- struct cdg *ca = inet_csk_ca(sk);
-
- return max(tcp_sk(sk)->snd_cwnd, ca->undo_cwnd);
-}
-
static void tcp_cdg_cwnd_event(struct sock *sk, const enum tcp_ca_event ev)
{
struct cdg *ca = inet_csk_ca(sk);
@@ -403,7 +393,7 @@ struct tcp_congestion_ops tcp_cdg __read_mostly = {
.cong_avoid = tcp_cdg_cong_avoid,
.cwnd_event = tcp_cdg_cwnd_event,
.pkts_acked = tcp_cdg_acked,
- .undo_cwnd = tcp_cdg_undo_cwnd,
+ .undo_cwnd = tcp_reno_undo_cwnd,
.ssthresh = tcp_cdg_ssthresh,
.release = tcp_cdg_release,
.init = tcp_cdg_init,
diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
index 57ae5b5ae643..78bfadfcf342 100644
--- a/net/ipv4/tcp_cubic.c
+++ b/net/ipv4/tcp_cubic.c
@@ -83,7 +83,6 @@ MODULE_PARM_DESC(hystart_ack_delta, "spacing between ack's indicating train (mse
struct bictcp {
u32 cnt; /* increase cwnd by 1 after ACKs */
u32 last_max_cwnd; /* last maximum snd_cwnd */
- u32 loss_cwnd; /* congestion window at last loss */
u32 last_cwnd; /* the last snd_cwnd */
u32 last_time; /* time when updated last_cwnd */
u32 bic_origin_point;/* origin point of bic function */
@@ -142,7 +141,6 @@ static void bictcp_init(struct sock *sk)
struct bictcp *ca = inet_csk_ca(sk);
bictcp_reset(ca);
- ca->loss_cwnd = 0;
if (hystart)
bictcp_hystart_reset(sk);
@@ -366,18 +364,9 @@ static u32 bictcp_recalc_ssthresh(struct sock *sk)
else
ca->last_max_cwnd = tp->snd_cwnd;
- ca->loss_cwnd = tp->snd_cwnd;
-
return max((tp->snd_cwnd * beta) / BICTCP_BETA_SCALE, 2U);
}
-static u32 bictcp_undo_cwnd(struct sock *sk)
-{
- struct bictcp *ca = inet_csk_ca(sk);
-
- return max(tcp_sk(sk)->snd_cwnd, ca->loss_cwnd);
-}
-
static void bictcp_state(struct sock *sk, u8 new_state)
{
if (new_state == TCP_CA_Loss) {
@@ -470,7 +459,7 @@ static struct tcp_congestion_ops cubictcp __read_mostly = {
.ssthresh = bictcp_recalc_ssthresh,
.cong_avoid = bictcp_cong_avoid,
.set_state = bictcp_state,
- .undo_cwnd = bictcp_undo_cwnd,
+ .undo_cwnd = tcp_reno_undo_cwnd,
.cwnd_event = bictcp_cwnd_event,
.pkts_acked = bictcp_acked,
.owner = THIS_MODULE,
diff --git a/net/ipv4/tcp_highspeed.c b/net/ipv4/tcp_highspeed.c
index 6d9879e93648..d1c33c91eadc 100644
--- a/net/ipv4/tcp_highspeed.c
+++ b/net/ipv4/tcp_highspeed.c
@@ -94,7 +94,6 @@ static const struct hstcp_aimd_val {
struct hstcp {
u32 ai;
- u32 loss_cwnd;
};
static void hstcp_init(struct sock *sk)
@@ -153,22 +152,14 @@ static u32 hstcp_ssthresh(struct sock *sk)
const struct tcp_sock *tp = tcp_sk(sk);
struct hstcp *ca = inet_csk_ca(sk);
- ca->loss_cwnd = tp->snd_cwnd;
/* Do multiplicative decrease */
return max(tp->snd_cwnd - ((tp->snd_cwnd * hstcp_aimd_vals[ca->ai].md) >> 8), 2U);
}
-static u32 hstcp_cwnd_undo(struct sock *sk)
-{
- const struct hstcp *ca = inet_csk_ca(sk);
-
- return max(tcp_sk(sk)->snd_cwnd, ca->loss_cwnd);
-}
-
static struct tcp_congestion_ops tcp_highspeed __read_mostly = {
.init = hstcp_init,
.ssthresh = hstcp_ssthresh,
- .undo_cwnd = hstcp_cwnd_undo,
+ .undo_cwnd = tcp_reno_undo_cwnd,
.cong_avoid = hstcp_cong_avoid,
.owner = THIS_MODULE,
diff --git a/net/ipv4/tcp_illinois.c b/net/ipv4/tcp_illinois.c
index 60352ff4f5a8..7c843578f233 100644
--- a/net/ipv4/tcp_illinois.c
+++ b/net/ipv4/tcp_illinois.c
@@ -48,7 +48,6 @@ struct illinois {
u32 end_seq; /* right edge of current RTT */
u32 alpha; /* Additive increase */
u32 beta; /* Muliplicative decrease */
- u32 loss_cwnd; /* cwnd on loss */
u16 acked; /* # packets acked by current ACK */
u8 rtt_above; /* average rtt has gone above threshold */
u8 rtt_low; /* # of rtts measurements below threshold */
@@ -297,18 +296,10 @@ static u32 tcp_illinois_ssthresh(struct sock *sk)
struct tcp_sock *tp = tcp_sk(sk);
struct illinois *ca = inet_csk_ca(sk);
- ca->loss_cwnd = tp->snd_cwnd;
/* Multiplicative decrease */
return max(tp->snd_cwnd - ((tp->snd_cwnd * ca->beta) >> BETA_SHIFT), 2U);
}
-static u32 tcp_illinois_cwnd_undo(struct sock *sk)
-{
- const struct illinois *ca = inet_csk_ca(sk);
-
- return max(tcp_sk(sk)->snd_cwnd, ca->loss_cwnd);
-}
-
/* Extract info for Tcp socket info provided via netlink. */
static size_t tcp_illinois_info(struct sock *sk, u32 ext, int *attr,
union tcp_cc_info *info)
@@ -336,7 +327,7 @@ static size_t tcp_illinois_info(struct sock *sk, u32 ext, int *attr,
static struct tcp_congestion_ops tcp_illinois __read_mostly = {
.init = tcp_illinois_init,
.ssthresh = tcp_illinois_ssthresh,
- .undo_cwnd = tcp_illinois_cwnd_undo,
+ .undo_cwnd = tcp_reno_undo_cwnd,
.cong_avoid = tcp_illinois_cong_avoid,
.set_state = tcp_illinois_state,
.get_info = tcp_illinois_info,
diff --git a/net/ipv4/tcp_nv.c b/net/ipv4/tcp_nv.c
index 6d650ed3cb59..1ff73982e28c 100644
--- a/net/ipv4/tcp_nv.c
+++ b/net/ipv4/tcp_nv.c
@@ -86,7 +86,6 @@ struct tcpnv {
* < 0 => less than 1 packet/RTT */
u8 available8;
u16 available16;
- u32 loss_cwnd; /* cwnd at last loss */
u8 nv_allow_cwnd_growth:1, /* whether cwnd can grow */
nv_reset:1, /* whether to reset values */
nv_catchup:1; /* whether we are growing because
@@ -121,7 +120,6 @@ static inline void tcpnv_reset(struct tcpnv *ca, struct sock *sk)
struct tcp_sock *tp = tcp_sk(sk);
ca->nv_reset = 0;
- ca->loss_cwnd = 0;
ca->nv_no_cong_cnt = 0;
ca->nv_rtt_cnt = 0;
ca->nv_last_rtt = 0;
@@ -177,19 +175,10 @@ static void tcpnv_cong_avoid(struct sock *sk, u32 ack, u32 acked)
static u32 tcpnv_recalc_ssthresh(struct sock *sk)
{
const struct tcp_sock *tp = tcp_sk(sk);
- struct tcpnv *ca = inet_csk_ca(sk);
- ca->loss_cwnd = tp->snd_cwnd;
return max((tp->snd_cwnd * nv_loss_dec_factor) >> 10, 2U);
}
-static u32 tcpnv_undo_cwnd(struct sock *sk)
-{
- struct tcpnv *ca = inet_csk_ca(sk);
-
- return max(tcp_sk(sk)->snd_cwnd, ca->loss_cwnd);
-}
-
static void tcpnv_state(struct sock *sk, u8 new_state)
{
struct tcpnv *ca = inet_csk_ca(sk);
@@ -446,7 +435,7 @@ static struct tcp_congestion_ops tcpnv __read_mostly = {
.ssthresh = tcpnv_recalc_ssthresh,
.cong_avoid = tcpnv_cong_avoid,
.set_state = tcpnv_state,
- .undo_cwnd = tcpnv_undo_cwnd,
+ .undo_cwnd = tcp_reno_undo_cwnd,
.pkts_acked = tcpnv_acked,
.get_info = tcpnv_get_info,
diff --git a/net/ipv4/tcp_scalable.c b/net/ipv4/tcp_scalable.c
index f2123075ce6e..addc122f8818 100644
--- a/net/ipv4/tcp_scalable.c
+++ b/net/ipv4/tcp_scalable.c
@@ -15,10 +15,6 @@
#define TCP_SCALABLE_AI_CNT 50U
#define TCP_SCALABLE_MD_SCALE 3
-struct scalable {
- u32 loss_cwnd;
-};
-
static void tcp_scalable_cong_avoid(struct sock *sk, u32 ack, u32 acked)
{
struct tcp_sock *tp = tcp_sk(sk);
@@ -36,23 +32,13 @@ static void tcp_scalable_cong_avoid(struct sock *sk, u32 ack, u32 acked)
static u32 tcp_scalable_ssthresh(struct sock *sk)
{
const struct tcp_sock *tp = tcp_sk(sk);
- struct scalable *ca = inet_csk_ca(sk);
-
- ca->loss_cwnd = tp->snd_cwnd;
return max(tp->snd_cwnd - (tp->snd_cwnd>>TCP_SCALABLE_MD_SCALE), 2U);
}
-static u32 tcp_scalable_cwnd_undo(struct sock *sk)
-{
- const struct scalable *ca = inet_csk_ca(sk);
-
- return max(tcp_sk(sk)->snd_cwnd, ca->loss_cwnd);
-}
-
static struct tcp_congestion_ops tcp_scalable __read_mostly = {
.ssthresh = tcp_scalable_ssthresh,
- .undo_cwnd = tcp_scalable_cwnd_undo,
+ .undo_cwnd = tcp_reno_undo_cwnd,
.cong_avoid = tcp_scalable_cong_avoid,
.owner = THIS_MODULE,
diff --git a/net/ipv4/tcp_veno.c b/net/ipv4/tcp_veno.c
index 76005d4b8dfc..6fcf482d611b 100644
--- a/net/ipv4/tcp_veno.c
+++ b/net/ipv4/tcp_veno.c
@@ -30,7 +30,6 @@ struct veno {
u32 basertt; /* the min of all Veno rtt measurements seen (in usec) */
u32 inc; /* decide whether to increase cwnd */
u32 diff; /* calculate the diff rate */
- u32 loss_cwnd; /* cwnd when loss occured */
};
/* There are several situations when we must "re-start" Veno:
@@ -194,7 +193,6 @@ static u32 tcp_veno_ssthresh(struct sock *sk)
const struct tcp_sock *tp = tcp_sk(sk);
struct veno *veno = inet_csk_ca(sk);
- veno->loss_cwnd = tp->snd_cwnd;
if (veno->diff < beta)
/* in "non-congestive state", cut cwnd by 1/5 */
return max(tp->snd_cwnd * 4 / 5, 2U);
@@ -203,17 +201,10 @@ static u32 tcp_veno_ssthresh(struct sock *sk)
return max(tp->snd_cwnd >> 1U, 2U);
}
-static u32 tcp_veno_cwnd_undo(struct sock *sk)
-{
- const struct veno *veno = inet_csk_ca(sk);
-
- return max(tcp_sk(sk)->snd_cwnd, veno->loss_cwnd);
-}
-
static struct tcp_congestion_ops tcp_veno __read_mostly = {
.init = tcp_veno_init,
.ssthresh = tcp_veno_ssthresh,
- .undo_cwnd = tcp_veno_cwnd_undo,
+ .undo_cwnd = tcp_reno_undo_cwnd,
.cong_avoid = tcp_veno_cong_avoid,
.pkts_acked = tcp_veno_pkts_acked,
.set_state = tcp_veno_state,
diff --git a/net/ipv4/tcp_yeah.c b/net/ipv4/tcp_yeah.c
index e6ff99c4bd3b..96e829b2e2fc 100644
--- a/net/ipv4/tcp_yeah.c
+++ b/net/ipv4/tcp_yeah.c
@@ -37,7 +37,6 @@ struct yeah {
u32 fast_count;
u32 pkts_acked;
- u32 loss_cwnd;
};
static void tcp_yeah_init(struct sock *sk)
@@ -220,22 +219,14 @@ static u32 tcp_yeah_ssthresh(struct sock *sk)
yeah->fast_count = 0;
yeah->reno_count = max(yeah->reno_count>>1, 2U);
- yeah->loss_cwnd = tp->snd_cwnd;
return max_t(int, tp->snd_cwnd - reduction, 2);
}
-static u32 tcp_yeah_cwnd_undo(struct sock *sk)
-{
- const struct yeah *yeah = inet_csk_ca(sk);
-
- return max(tcp_sk(sk)->snd_cwnd, yeah->loss_cwnd);
-}
-
static struct tcp_congestion_ops tcp_yeah __read_mostly = {
.init = tcp_yeah_init,
.ssthresh = tcp_yeah_ssthresh,
- .undo_cwnd = tcp_yeah_cwnd_undo,
+ .undo_cwnd = tcp_reno_undo_cwnd,
.cong_avoid = tcp_yeah_cong_avoid,
.set_state = tcp_vegas_state,
.cwnd_event = tcp_vegas_cwnd_event,
--
2.14.0.rc1.383.gd1ce394fe2-goog
^ permalink raw reply related
* RE: [PATCH V6 net-next 0/8] Hisilicon Network Subsystem 3 Ethernet Driver
From: Salil Mehta @ 2017-08-04 3:49 UTC (permalink / raw)
To: David Miller
Cc: Zhuangyuzeng (Yisen), huangdaode, lipeng (Y),
mehta.salil.lnk@gmail.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org,
Linuxarm
In-Reply-To: <20170803.150937.678353968962234413.davem@davemloft.net>
Thanks a ton, Dave!
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Thursday, August 03, 2017 11:10 PM
> To: Salil Mehta
> Cc: Zhuangyuzeng (Yisen); huangdaode; lipeng (Y);
> mehta.salil.lnk@gmail.com; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH V6 net-next 0/8] Hisilicon Network Subsystem 3
> Ethernet Driver
>
> From: Salil Mehta <salil.mehta@huawei.com>
> Date: Wed, 2 Aug 2017 16:59:44 +0100
>
> > This patch-set contains the support of the HNS3 (Hisilicon Network
> Subsystem 3)
> > Ethernet driver for hip08 family of SoCs and future upcoming SoCs.
> ...
>
> Series applied, thanks.
^ permalink raw reply
* Re: [RFC PATCH 4/6] net: sockmap with sk redirect support
From: Tom Herbert @ 2017-08-04 4:22 UTC (permalink / raw)
To: John Fastabend
Cc: David S. Miller, Alexei Starovoitov,
Linux Kernel Network Developers, Daniel Borkmann
In-Reply-To: <20170803233756.12107.54982.stgit@john-Precision-Tower-5810>
On Thu, Aug 3, 2017 at 4:37 PM, John Fastabend <john.fastabend@gmail.com> wrote:
> Recently we added a new map type called dev map used to forward XDP
> packets between ports (6093ec2dc313). This patches introduces a
> similar notion for sockets.
>
> A sockmap allows users to add participating sockets to a map. When
> sockets are added to the map enough context is stored with the
> map entry to use the entry with a new helper
>
> bpf_sk_redirect_map(map, key, flags)
>
> This helper (analogous to bpf_redirect_map in XDP) is given the map
> and an entry in the map. When called from a sockmap program, discussed
> below, the skb will be sent on the socket using skb_send_sock().
>
> With the above we need a bpf program to call the helper from that will
> then implement the send logic. The initial site implemented in this
> series is the recv_sock hook. For this to work we implemented a map
> attach command to add attributes to a map. In sockmap we add two
> programs a parse program and a verdict program. The parse program
> uses strparser to build messages and pass them to the verdict program.
> The parse program usese normal strparser semantics. The verdict
> program is of type SOCKET_FILTER.
>
> The verdict program returns a verdict BPF_OK, BPF_DROP, BPF_REDIRECT.
> When BPF_REDIRECT is returned, expected when bpf program uses
> bpf_sk_redirect_map(), the sockmap logic will consult per cpu variables
> set by the helper routine and pull the sock entry out of the sock map.
> This pattern follows the existing redirect logic in cls and xdp
> programs.
>
Hi John,
I'm a bit confused. If the verdict program bpf_mux then? I don't see
any use of BPF_OK,DROP, or REDIRECT. I assume I'm missing something.
Tom
> This gives the flow,
>
> recv_sock -> str_parser (parse_prog) -> verdict_prog -> skb_send_sock
>
> As an example use case a message based load balancer may use specific
> logic in the verdict program to select the sock to send on.
>
> Example and sample programs are provided in future patches that
> hopefully illustrate the user interfaces.
>
> TBD: bpf program refcnt'ing needs to be cleaned up, some additional
> cleanup in a few error paths, publish performance numbers and some
> self tests.
>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> include/linux/bpf.h | 11 +
> include/linux/bpf_types.h | 1
> include/uapi/linux/bpf.h | 13 +
> kernel/bpf/Makefile | 2
> kernel/bpf/helpers.c | 20 +
> kernel/bpf/sockmap.c | 623 +++++++++++++++++++++++++++++++++++++++++++++
> kernel/bpf/syscall.c | 41 +++
> net/core/filter.c | 51 ++++
> 8 files changed, 759 insertions(+), 3 deletions(-)
> create mode 100644 kernel/bpf/sockmap.c
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 6353c74..9ce6aa0 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -15,6 +15,8 @@
> #include <linux/err.h>
> #include <linux/rbtree_latch.h>
>
> +#include <net/sock.h>
> +
> struct perf_event;
> struct bpf_map;
>
> @@ -29,6 +31,9 @@ struct bpf_map_ops {
> /* funcs callable from userspace and from eBPF programs */
> void *(*map_lookup_elem)(struct bpf_map *map, void *key);
> int (*map_update_elem)(struct bpf_map *map, void *key, void *value, u64 flags);
> + int (*map_ctx_update_elem)(struct bpf_sock_ops_kern *skops,
> + struct bpf_map *map,
> + void *key, u64 flags, u64 map_flags);
> int (*map_delete_elem)(struct bpf_map *map, void *key);
>
> /* funcs called by prog_array and perf_event_array map */
> @@ -37,6 +42,7 @@ struct bpf_map_ops {
> void (*map_fd_put_ptr)(void *ptr);
> u32 (*map_gen_lookup)(struct bpf_map *map, struct bpf_insn *insn_buf);
> u32 (*map_fd_sys_lookup_elem)(void *ptr);
> + int (*map_attach)(struct bpf_map *map, struct bpf_prog *p1, struct bpf_prog *p2);
> };
>
> struct bpf_map {
> @@ -321,6 +327,7 @@ static inline void bpf_long_memcpy(void *dst, const void *src, u32 size)
>
> /* Map specifics */
> struct net_device *__dev_map_lookup_elem(struct bpf_map *map, u32 key);
> +struct sock *__sock_map_lookup_elem(struct bpf_map *map, u32 key);
> void __dev_map_insert_ctx(struct bpf_map *map, u32 index);
> void __dev_map_flush(struct bpf_map *map);
>
> @@ -378,9 +385,13 @@ static inline void __dev_map_flush(struct bpf_map *map)
> }
> #endif /* CONFIG_BPF_SYSCALL */
>
> +inline struct sock *do_sk_redirect_map(void);
> +inline u64 get_sk_redirect_flags(void);
> +
> /* verifier prototypes for helper functions called from eBPF programs */
> extern const struct bpf_func_proto bpf_map_lookup_elem_proto;
> extern const struct bpf_func_proto bpf_map_update_elem_proto;
> +extern const struct bpf_func_proto bpf_map_ctx_update_elem_proto;
> extern const struct bpf_func_proto bpf_map_delete_elem_proto;
>
> extern const struct bpf_func_proto bpf_get_prandom_u32_proto;
> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> index b1e1035..930be52 100644
> --- a/include/linux/bpf_types.h
> +++ b/include/linux/bpf_types.h
> @@ -37,4 +37,5 @@
> BPF_MAP_TYPE(BPF_MAP_TYPE_HASH_OF_MAPS, htab_of_maps_map_ops)
> #ifdef CONFIG_NET
> BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP, dev_map_ops)
> +BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKMAP, sock_map_ops)
> #endif
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 1106a8c..a89e831 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -105,6 +105,7 @@ enum bpf_map_type {
> BPF_MAP_TYPE_ARRAY_OF_MAPS,
> BPF_MAP_TYPE_HASH_OF_MAPS,
> BPF_MAP_TYPE_DEVMAP,
> + BPF_MAP_TYPE_SOCKMAP,
> };
>
> enum bpf_prog_type {
> @@ -129,6 +130,7 @@ enum bpf_attach_type {
> BPF_CGROUP_INET_EGRESS,
> BPF_CGROUP_INET_SOCK_CREATE,
> BPF_CGROUP_SOCK_OPS,
> + BPF_SOCKMAP_INGRESS,
> __MAX_BPF_ATTACH_TYPE
> };
>
> @@ -205,6 +207,7 @@ enum bpf_attach_type {
> __u32 attach_bpf_fd; /* eBPF program to attach */
> __u32 attach_type;
> __u32 attach_flags;
> + __u32 attach_bpf_fd2;
> };
>
> struct { /* anonymous struct used by BPF_PROG_TEST_RUN command */
> @@ -598,7 +601,9 @@ enum bpf_attach_type {
> FN(set_hash), \
> FN(setsockopt), \
> FN(skb_adjust_room), \
> - FN(redirect_map),
> + FN(redirect_map), \
> + FN(sk_redirect_map), \
> + FN(map_ctx_update_elem), \
>
> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> * function eBPF program intends to call
> @@ -735,6 +740,12 @@ struct xdp_md {
> __u32 data_end;
> };
>
> +enum sk_action {
> + SK_ABORTED = 0,
> + SK_DROP,
> + SK_REDIRECT,
> +};
> +
> #define BPF_TAG_SIZE 8
>
> struct bpf_prog_info {
> diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
> index 48e9270..3089102 100644
> --- a/kernel/bpf/Makefile
> +++ b/kernel/bpf/Makefile
> @@ -3,7 +3,7 @@ obj-y := core.o
> obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o
> obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o lpm_trie.o map_in_map.o
> ifeq ($(CONFIG_NET),y)
> -obj-$(CONFIG_BPF_SYSCALL) += devmap.o
> +obj-$(CONFIG_BPF_SYSCALL) += devmap.o sockmap.o
> endif
> ifeq ($(CONFIG_PERF_EVENTS),y)
> obj-$(CONFIG_BPF_SYSCALL) += stackmap.o
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 3d24e23..feb38e0 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -43,6 +43,26 @@
> .arg2_type = ARG_PTR_TO_MAP_KEY,
> };
>
> +BPF_CALL_5(bpf_ctx_map_update_elem, struct bpf_sock_ops_kern *, bpf_sock,
> + struct bpf_map *, map, void *, key, u64, flags, u64, map_flags)
> +{
> + WARN_ON_ONCE(!rcu_read_lock_held());
> + return map->ops->map_ctx_update_elem(bpf_sock, map, key,
> + flags, map_flags);
> +}
> +
> +const struct bpf_func_proto bpf_map_ctx_update_elem_proto = {
> + .func = bpf_ctx_map_update_elem,
> + .gpl_only = false,
> + .pkt_access = true,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_CTX,
> + .arg2_type = ARG_CONST_MAP_PTR,
> + .arg3_type = ARG_PTR_TO_MAP_KEY,
> + .arg4_type = ARG_ANYTHING,
> + .arg5_type = ARG_ANYTHING,
> +};
> +
> BPF_CALL_4(bpf_map_update_elem, struct bpf_map *, map, void *, key,
> void *, value, u64, flags)
> {
> diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
> new file mode 100644
> index 0000000..9e88c32
> --- /dev/null
> +++ b/kernel/bpf/sockmap.c
> @@ -0,0 +1,623 @@
> +/* Copyright (c) 2017 Covalent IO, Inc. http://covalent.io
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of version 2 of the GNU General Public
> + * License as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + */
> +
> +/* A BPF sock_map is used to store sock objects. This is primarly used
> + * for doing socket redirect with BPF helper routines.
> + *
> + * A sock map may have two BPF programs attached to it, a program used
> + * to parse packets and a program to provide a verdict and redirect
> + * decision on the packet. If no BPF parse program is provided it is
> + * assumed that every skb is a "message" (skb->len). Otherwise the
> + * parse program is attached to strparser and used to build messages
> + * that may span multiple skbs. The verdict program will either select
> + * a socket to send/receive the skb on or provide the drop code indicating
> + * the skb should be dropped. More actions may be added later as needed.
> + * The default program will drop packets.
> + *
> + * For reference this program is similar to devmap used in XDP context
> + * reviewing these together may be useful. For a set of examples and
> + * test codes using this map please review ./samples/bpf/sockmap/ here
> + * you can find common usages such as a socket level load balancer and
> + * cgroup integration.
> + */
> +#include <linux/bpf.h>
> +#include <linux/jhash.h>
> +#include <linux/filter.h>
> +#include <net/sock.h>
> +#include <linux/rculist_nulls.h>
> +#include "percpu_freelist.h"
> +#include "bpf_lru_list.h"
> +#include "map_in_map.h"
> +
> +#include <linux/errno.h>
> +#include <linux/file.h>
> +#include <linux/in.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/net.h>
> +#include <linux/rculist.h>
> +#include <linux/skbuff.h>
> +#include <linux/socket.h>
> +#include <linux/workqueue.h>
> +#include <linux/list.h>
> +#include <linux/bpf.h>
> +#include <net/strparser.h>
> +#include <net/netns/generic.h>
> +#include <net/sock.h>
> +
> +struct bpf_stab {
> + struct bpf_map map;
> + struct sock **sock_map;
> + struct bpf_prog *bpf_parse;
> + struct bpf_prog *bpf_mux;
> +};
> +
> +struct smap_psock {
> + struct rcu_head rcu;
> +
> + /* datapath variables used under sock lock */
> + struct sk_buff_head rxqueue;
> +
> + bool strp_enabled;
> +
> + /* datapath error path cache across tx work invocations */
> + int save_rem;
> + int save_off;
> + struct sk_buff *save_skb;
> + u32 tx_stopped : 1;
> +
> + struct strparser strp;
> + struct bpf_prog *bpf_parse;
> + struct bpf_prog *bpf_mux;
> + struct bpf_map *map;
> +
> + /* Back reference to the file descriptor of the sock */
> + int key;
> + struct sock *sock;
> +
> + struct work_struct tx_work;
> +
> + void (*save_data_ready)(struct sock *sk);
> + void (*save_write_space)(struct sock *sk);
> + void (*save_state_change)(struct sock *sk);
> +};
> +
> +static inline struct smap_psock *smap_psock_sk(const struct sock *sk)
> +{
> + return (struct smap_psock *)sk->sk_user_data;
> +}
> +
> +static int smap_mux_func(struct smap_psock *psock, struct sk_buff *skb)
> +{
> + struct bpf_prog *prog = psock->bpf_mux;
> + int rc;
> +
> + if (unlikely(!prog))
> + return 0;
> +
> + skb->sk = psock->sock;
> + rc = (*prog->bpf_func)(skb, prog->insnsi);
> + skb->sk = NULL;
> +
> + return rc;
> +}
> +
> +static struct smap_psock *smap_peers_get(struct smap_psock *psock,
> + struct sk_buff *skb)
> +{
> + struct sock *sock;
> + int rc;
> +
> + rc = smap_mux_func(psock, skb);
> + if (unlikely(rc < 0))
> + return NULL;
> +
> + sock = do_sk_redirect_map();
> + if (unlikely(!sock))
> + return NULL;
> +
> + return smap_psock_sk(sock);
> +}
> +
> +static void smap_report_sk_error(struct smap_psock *psock, int err)
> +{
> + struct sock *sk = psock->sock;
> +
> + sk->sk_err = err;
> + sk->sk_error_report(sk);
> +}
> +
> +static int sock_map_delete_elem(struct bpf_map *map, void *key);
> +
> +static void smap_state_change(struct sock *sk)
> +{
> + struct smap_psock *psock = smap_psock_sk(sk);
> +
> + /* Allowing transitions into established an syn_recv states allows
> + * for early binding sockets to a smap object before the connection
> + * is established. All other transitions indicate the connection is
> + * being torn down so tear down the smap socket.
> + */
> + switch (sk->sk_state) {
> + case TCP_SYN_RECV:
> + case TCP_ESTABLISHED:
> + break;
> + case TCP_CLOSE_WAIT:
> + case TCP_CLOSING:
> + case TCP_LAST_ACK:
> + case TCP_FIN_WAIT1:
> + case TCP_FIN_WAIT2:
> + case TCP_LISTEN:
> + break;
> + case TCP_CLOSE:
> + sock_map_delete_elem(psock->map, &psock->key);
> + break;
> + default:
> + smap_report_sk_error(psock, EPIPE);
> + break;
> + }
> +}
> +
> +static void smap_tx_work(struct work_struct *w);
> +
> +void schedule_writer(struct smap_psock *psock)
> +{
> + schedule_work(&psock->tx_work);
> +}
> +
> +static int smap_tx_writer(struct smap_psock *peer)
> +{
> + schedule_writer(peer);
> + return 0;
> +}
> +
> +static void smap_read_sock_strparser(struct strparser *strp,
> + struct sk_buff *skb)
> +{
> + struct smap_psock *psock = container_of(strp,
> + struct smap_psock, strp);
> + struct smap_psock *peer;
> +
> + /* TBD useful dbg, add trace here with output sock index or drop */
> + rcu_read_lock();
> + peer = smap_peers_get(psock, skb);
> + if (unlikely(!peer)) {
> + kfree_skb(skb);
> + goto out;
> + }
> +
> + skb_queue_tail(&peer->rxqueue, skb);
> + smap_tx_writer(peer);
> +out:
> + rcu_read_unlock();
> +}
> +
> +/* Called with lock held on socket */
> +static void smap_data_ready(struct sock *sk)
> +{
> + struct smap_psock *psock;
> +
> + read_lock_bh(&sk->sk_callback_lock);
> +
> + psock = smap_psock_sk(sk);
> + if (likely(psock))
> + strp_data_ready(&psock->strp);
> +
> + read_unlock_bh(&sk->sk_callback_lock);
> +}
> +
> +static void smap_tx_work(struct work_struct *w)
> +{
> + struct smap_psock *psock;
> + struct sk_buff *skb;
> + int rem, off, n;
> +
> + psock = container_of(w, struct smap_psock, tx_work);
> + if (unlikely(psock->tx_stopped))
> + return;
> +
> + if (psock->save_skb) {
> + skb = psock->save_skb;
> + rem = psock->save_rem;
> + off = psock->save_off;
> + psock->save_skb = NULL;
> + goto start;
> + }
> +
> + while ((skb = skb_dequeue(&psock->rxqueue))) {
> + rem = skb->len;
> + off = 0;
> +start:
> + do {
> + n = skb_send_sock(psock->sock, skb, off, rem);
> + if (n <= 0) {
> + if (n == -EAGAIN) {
> + /* Save state to try again when
> + * there's write space on the
> + * socket.
> + */
> + psock->save_skb = skb;
> + psock->save_rem = rem;
> + psock->save_off = off;
> + break;
> + }
> +
> + /* Got a hard error or socket had
> + * been closed somehow. Report this
> + * on the transport socket.
> + */
> + smap_report_sk_error(psock, n ? -n : EPIPE);
> + psock->tx_stopped = 1;
> + break;
> + }
> + rem -= n;
> + off += n;
> + } while (rem);
> + }
> +}
> +
> +static void smap_write_space(struct sock *sk)
> +{
> + struct smap_psock *psock = smap_psock_sk(sk);
> +
> + schedule_writer(psock);
> +}
> +
> +static void smap_stop_sock(struct smap_psock *psock, bool destroy)
> +{
> + struct sock *sk = psock->sock;
> +
> + write_lock_bh(&sk->sk_callback_lock);
> + if (psock->strp_enabled) {
> + sk->sk_data_ready = psock->save_data_ready;
> + sk->sk_write_space = psock->save_write_space;
> + sk->sk_state_change = psock->save_state_change;
> + strp_stop(&psock->strp);
> + }
> +
> + if (destroy)
> + sk->sk_user_data = NULL;
> + write_unlock_bh(&sk->sk_callback_lock);
> +
> + if (psock->strp_enabled)
> + strp_done(&psock->strp);
> + psock->strp_enabled = false;
> +}
> +
> +static void smap_destroy_psock(struct rcu_head *rcu)
> +{
> + struct smap_psock *psock = container_of(rcu,
> + struct smap_psock, rcu);
> +
> + smap_stop_sock(psock, true);
> + cancel_work_sync(&psock->tx_work);
> + __skb_queue_purge(&psock->rxqueue);
> + sock_put(psock->sock);
> + kfree(psock);
> +}
> +
> +static void smap_release_proxy(struct sock *sock)
> +{
> + struct smap_psock *psock = smap_psock_sk(sock);
> +
> + call_rcu(&psock->rcu, smap_destroy_psock);
> +}
> +
> +static int smap_parse_func_strparser(struct strparser *strp,
> + struct sk_buff *skb)
> +{
> + struct smap_psock *psock = container_of(strp,
> + struct smap_psock, strp);
> + struct bpf_prog *prog = psock->bpf_parse;
> +
> + if (unlikely(!prog))
> + return skb->len;
> +
> + return (*prog->bpf_func)(skb, prog->insnsi);
> +}
> +
> +
> +static int smap_read_sock_done(struct strparser *strp, int err)
> +{
> + return err;
> +}
> +
> +static int smap_init_sock(struct smap_psock *psock,
> + struct sock *sock)
> +{
> + struct strp_callbacks cb;
> + int err;
> +
> + cb.rcv_msg = smap_read_sock_strparser;
> + cb.abort_parser = NULL;
> + cb.parse_msg = smap_parse_func_strparser;
> + cb.read_sock_done = smap_read_sock_done;
> +
> + err = strp_init(&psock->strp, sock, &cb);
> + if (err)
> + return -EINVAL;
> + return 0;
> +}
> +
> +static void smap_init_progs(struct smap_psock *psock, struct bpf_stab *stab)
> +{
> + /* TBD need prog_put and gets here to avoid programs leaving
> + * us or something in attach
> + */
> + if (psock->bpf_mux != stab->bpf_mux)
> + psock->bpf_mux = stab->bpf_mux;
> +
> + if (psock->bpf_parse != stab->bpf_parse)
> + psock->bpf_parse = stab->bpf_parse;
> +}
> +
> +static int smap_start_sock(struct smap_psock *psock, struct sock *sk)
> +{
> + int err = 0;
> +
> + write_lock_bh(&sk->sk_callback_lock);
> + /* only start socket if it is not already running */
> + if (psock->save_data_ready) {
> + err = -EINVAL;
> + goto out;
> + }
> + psock->save_data_ready = sk->sk_data_ready;
> + psock->save_write_space = sk->sk_write_space;
> + psock->save_state_change = sk->sk_state_change;
> + sk->sk_data_ready = smap_data_ready;
> + sk->sk_write_space = smap_write_space;
> + sk->sk_state_change = smap_state_change;
> +out:
> + write_unlock_bh(&sk->sk_callback_lock);
> + return err;
> +}
> +
> +static struct smap_psock *smap_init_psock(struct sock *sock,
> + struct bpf_stab *stab)
> +{
> + struct smap_psock *psock;
> +
> + psock = kmalloc(sizeof(struct smap_psock), GFP_ATOMIC);
> + if (!psock)
> + return ERR_PTR(-ENOMEM);
> +
> + memset(psock, 0, sizeof(struct smap_psock));
> + smap_init_progs(psock, stab);
> + psock->sock = sock;
> +
> + skb_queue_head_init(&psock->rxqueue);
> + INIT_WORK(&psock->tx_work, smap_tx_work);
> +
> + write_lock_bh(&sock->sk_callback_lock);
> + sock->sk_user_data = psock;
> + write_unlock_bh(&sock->sk_callback_lock);
> +
> + sock_hold(sock);
> + return psock;
> +}
> +
> +#define SOCK_MAP_STRPARSER 0x01
> +/* BPF map logic */
> +static struct bpf_map *sock_map_alloc(union bpf_attr *attr)
> +{
> + struct bpf_stab *stab;
> + int err = -EINVAL;
> + u64 cost;
> +
> + /* check sanity of attributes */
> + if (attr->max_entries == 0 || attr->key_size != 4 ||
> + attr->value_size != 4 || attr->map_flags)
> + return ERR_PTR(-EINVAL);
> +
> + /* if value_size is bigger, the user space won't be able to
> + * access the elements.
> + */
> + if (attr->value_size > KMALLOC_MAX_SIZE)
> + return ERR_PTR(-E2BIG);
> +
> + stab = kzalloc(sizeof(*stab), GFP_USER);
> + if (!stab)
> + return ERR_PTR(-ENOMEM);
> +
> + /* mandatory map attributes */
> + stab->map.map_type = attr->map_type;
> + stab->map.key_size = attr->key_size;
> + stab->map.value_size = attr->value_size;
> + stab->map.max_entries = attr->max_entries;
> + stab->map.map_flags = attr->map_flags;
> +
> +
> + /* make sure page count doesn't overflow */
> + cost = (u64) stab->map.max_entries * sizeof(struct sock *) +
> + sizeof(struct socket *);
> + stab->map.pages = round_up(cost, PAGE_SIZE) >> PAGE_SHIFT;
> +
> + err = -ENOMEM;
> +
> + /* if map size is larger than memlock limit, reject it early */
> + err = bpf_map_precharge_memlock(stab->map.pages);
> + if (err)
> + goto free_stab;
> +
> + stab->sock_map = bpf_map_area_alloc(stab->map.max_entries *
> + sizeof(struct sock *));
> + if (!stab->sock_map)
> + goto free_stab;
> +
> + return &stab->map;
> + /* TBD release progs on errors */
> +free_stab:
> + kfree(stab);
> + return ERR_PTR(err);
> +}
> +
> +static void sock_map_free(struct bpf_map *map)
> +{
> + struct bpf_stab *stab = container_of(map, struct bpf_stab, map);
> + int i;
> +
> + synchronize_rcu();
> +
> + for (i = 0; i < stab->map.max_entries; i++) {
> + struct sock *sock;
> +
> + sock = stab->sock_map[i];
> + if (!sock)
> + continue;
> +
> + smap_release_proxy(sock);
> + }
> +
> + bpf_map_area_free(stab->sock_map);
> + if (stab->bpf_mux)
> + bpf_prog_put(stab->bpf_mux);
> + if (stab->bpf_parse)
> + bpf_prog_put(stab->bpf_mux);
> + kfree(stab);
> +}
> +
> +static int sock_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
> +{
> + struct bpf_stab *stab = container_of(map, struct bpf_stab, map);
> + u32 i = key ? *(u32 *)key : U32_MAX;
> + u32 *next = (u32 *)next_key;
> +
> + if (i >= stab->map.max_entries) {
> + *next = 0;
> + return 0;
> + }
> +
> + if (i == stab->map.max_entries - 1)
> + return -ENOENT;
> +
> + *next = i + 1;
> + return 0;
> +}
> +
> +struct sock *__sock_map_lookup_elem(struct bpf_map *map, u32 key)
> +{
> + struct bpf_stab *stab = container_of(map, struct bpf_stab, map);
> +
> + if (key >= map->max_entries)
> + return NULL;
> +
> + return stab->sock_map[key];
> +}
> +
> +static void *sock_map_lookup_elem(struct bpf_map *map, void *key)
> +{
> + struct bpf_stab *stab = container_of(map, struct bpf_stab, map);
> + struct sock *sock;
> + u32 i = *(u32 *)key;
> +
> + if (i >= map->max_entries)
> + return NULL;
> +
> + sock = stab->sock_map[i];
> + return NULL;
> +}
> +
> +static int sock_map_delete_elem(struct bpf_map *map, void *key)
> +{
> + struct bpf_stab *stab = container_of(map, struct bpf_stab, map);
> + struct sock *sock;
> + int k = *(u32 *)key;
> +
> + if (k >= map->max_entries)
> + return -EINVAL;
> +
> + sock = stab->sock_map[k];
> + if (!sock)
> + return -EINVAL;
> +
> + smap_release_proxy(sock);
> + return 0;
> +}
> +
> +static int sock_map_update_elem(struct bpf_sock_ops_kern *skops,
> + struct bpf_map *map,
> + void *key, u64 flags, u64 map_flags)
> +{
> + struct bpf_stab *stab = container_of(map, struct bpf_stab, map);
> + struct sock *old_sock, *sock;
> + struct smap_psock *psock = NULL;
> + u32 i = *(u32 *)key;
> + bool update = false;
> +
> + if (unlikely(flags > BPF_EXIST))
> + return -EINVAL;
> +
> + if (unlikely(i >= stab->map.max_entries))
> + return -E2BIG;
> +
> + if (unlikely(map_flags > SOCK_MAP_STRPARSER))
> + return -EINVAL;
> +
> + if (flags == BPF_EXIST || flags == BPF_ANY) {
> + sock = rcu_dereference(stab->sock_map[i]);
> +
> + if (!sock && flags == BPF_EXIST) {
> + return -ENOENT;
> + } else if (sock && sock != skops->sk) {
> + return -EINVAL;
> + } else if (sock) {
> + psock = smap_psock_sk(sock);
> + update = true;
> + }
> + }
> +
> + if (!psock) {
> + sock = skops->sk;
> + psock = smap_init_psock(sock, stab);
> + if (IS_ERR(psock))
> + return PTR_ERR(psock);
> + psock->key = i;
> + psock->map = map;
> + }
> +
> + if (map_flags & SOCK_MAP_STRPARSER) {
> + smap_start_sock(psock, sock);
> + smap_init_progs(psock, stab);
> + smap_init_sock(psock, sock);
> + psock->strp_enabled = true;
> + } else if (update) {
> + smap_stop_sock(psock, false);
> + }
> +
> + if (!update) {
> + old_sock = xchg(&stab->sock_map[i], skops->sk);
> + if (old_sock)
> + smap_release_proxy(old_sock);
> + }
> +
> + return 0;
> +}
> +
> +static int sock_map_attach_prog(struct bpf_map *map,
> + struct bpf_prog *parse, struct bpf_prog *mux)
> +{
> + struct bpf_stab *stab = container_of(map, struct bpf_stab, map);
> +
> + stab->bpf_parse = parse;
> + stab->bpf_mux = mux;
> + return 0;
> +}
> +
> +const struct bpf_map_ops sock_map_ops = {
> + .map_alloc = sock_map_alloc,
> + .map_free = sock_map_free,
> + .map_get_next_key = sock_map_get_next_key,
> + .map_lookup_elem = sock_map_lookup_elem,
> + .map_ctx_update_elem = sock_map_update_elem,
> + .map_delete_elem = sock_map_delete_elem,
> + .map_attach = sock_map_attach_prog,
> +};
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 6c772ad..e4f48f5 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1045,7 +1045,40 @@ static int bpf_obj_get(const union bpf_attr *attr)
>
> #ifdef CONFIG_CGROUP_BPF
>
> -#define BPF_PROG_ATTACH_LAST_FIELD attach_flags
> +#define BPF_PROG_ATTACH_LAST_FIELD attach_bpf_fd2
> +
> +static int sockmap_get_from_fd(const union bpf_attr *attr, int ptype)
> +{
> + struct bpf_prog *prog1, *prog2;
> + struct bpf_map *map;
> + int err;
> +
> + map = bpf_map_get_with_uref(attr->target_fd);
> + if (IS_ERR(map))
> + return PTR_ERR(map);
> +
> + if (!map->ops->map_attach)
> + return -EOPNOTSUPP;
> +
> + prog1 = bpf_prog_get_type(attr->attach_bpf_fd, ptype);
> + if (IS_ERR(prog1))
> + return PTR_ERR(prog1);
> +
> + prog2 = bpf_prog_get_type(attr->attach_bpf_fd2, ptype);
> + if (IS_ERR(prog2)) {
> + bpf_prog_put(prog1);
> + return PTR_ERR(prog2);
> + }
> +
> + err = map->ops->map_attach(map, prog1, prog2);
> + if (err) {
> + bpf_prog_put(prog1);
> + bpf_prog_put(prog2);
> + return PTR_ERR(map);
> + }
> +
> + return err;
> +}
>
> static int bpf_prog_attach(const union bpf_attr *attr)
> {
> @@ -1074,10 +1107,16 @@ static int bpf_prog_attach(const union bpf_attr *attr)
> case BPF_CGROUP_SOCK_OPS:
> ptype = BPF_PROG_TYPE_SOCK_OPS;
> break;
> + case BPF_SOCKMAP_INGRESS:
> + ptype = BPF_PROG_TYPE_SOCKET_FILTER;
> + break;
> default:
> return -EINVAL;
> }
>
> + if (attr->attach_type == BPF_SOCKMAP_INGRESS)
> + return sockmap_get_from_fd(attr, ptype);
> +
> prog = bpf_prog_get_type(attr->attach_bpf_fd, ptype);
> if (IS_ERR(prog))
> return PTR_ERR(prog);
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 7e97086..2644f2d 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -1845,6 +1845,51 @@ int skb_do_redirect(struct sk_buff *skb)
> .arg3_type = ARG_ANYTHING,
> };
>
> +BPF_CALL_3(bpf_sk_redirect_map, struct bpf_map *, map, u32, key, u64, flags)
> +{
> + struct redirect_info *ri = this_cpu_ptr(&redirect_info);
> +
> + ri->ifindex = key;
> + ri->flags = flags;
> + ri->map = map;
> +
> + return SK_REDIRECT;
> +}
> +
> +inline struct sock *do_sk_redirect_map(void)
> +{
> + struct redirect_info *ri = this_cpu_ptr(&redirect_info);
> + struct sock *sk = NULL;
> +
> + if (ri->map) {
> + sk = __sock_map_lookup_elem(ri->map, ri->ifindex);
> +
> + ri->ifindex = 0;
> + ri->map = NULL;
> + /* we do not clear flags for future lookup */
> + }
> +
> + return sk;
> +}
> +EXPORT_SYMBOL(do_sk_redirect_map);
> +
> +inline u64 get_sk_redirect_flags(void)
> +{
> + struct redirect_info *ri = this_cpu_ptr(&redirect_info);
> +
> + return ri->flags;
> +}
> +EXPORT_SYMBOL(get_sk_redirect_flags);
> +
> +static const struct bpf_func_proto bpf_sk_redirect_map_proto = {
> + .func = bpf_sk_redirect_map,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_CONST_MAP_PTR,
> + .arg2_type = ARG_ANYTHING,
> + .arg3_type = ARG_ANYTHING,
> +};
> +
> BPF_CALL_1(bpf_get_cgroup_classid, const struct sk_buff *, skb)
> {
> return task_get_classid(skb);
> @@ -3090,6 +3135,10 @@ static unsigned long bpf_xdp_copy(void *dst_buff, const void *src_buff,
> return &bpf_get_socket_cookie_proto;
> case BPF_FUNC_get_socket_uid:
> return &bpf_get_socket_uid_proto;
> + case BPF_FUNC_sk_redirect_map:
> + return &bpf_sk_redirect_map_proto;
> + case BPF_FUNC_map_ctx_update_elem:
> + return &bpf_map_ctx_update_elem_proto;
> default:
> return bpf_base_func_proto(func_id);
> }
> @@ -3214,6 +3263,8 @@ static unsigned long bpf_xdp_copy(void *dst_buff, const void *src_buff,
> switch (func_id) {
> case BPF_FUNC_setsockopt:
> return &bpf_setsockopt_proto;
> + case BPF_FUNC_map_ctx_update_elem:
> + return &bpf_map_ctx_update_elem_proto;
> default:
> return bpf_base_func_proto(func_id);
> }
>
^ permalink raw reply
* [PATCH net-next] net: dsa: User per-cpu 64-bit statistics
From: Florian Fainelli @ 2017-08-04 4:33 UTC (permalink / raw)
To: netdev
Cc: davem, Florian Fainelli, Andrew Lunn, Vivien Didelot,
David S. Miller, open list
During testing with a background iperf pushing 1Gbit/sec worth of
traffic and having both ifconfig and ethtool collect statistics, we
could see quite frequent deadlocks. Convert the often accessed DSA slave
network devices statistics to per-cpu 64-bit statistics to remove these
deadlocks and provide fast efficient statistics updates.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/dsa.c | 10 +++++---
net/dsa/dsa_priv.h | 2 +-
net/dsa/slave.c | 72 +++++++++++++++++++++++++++++++++++++++---------------
3 files changed, 59 insertions(+), 25 deletions(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 0ba842c08dd3..a91e520e735f 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -190,6 +190,7 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
{
struct dsa_switch_tree *dst = dev->dsa_ptr;
struct sk_buff *nskb = NULL;
+ struct pcpu_sw_netstats *s;
struct dsa_slave_priv *p;
if (unlikely(dst == NULL)) {
@@ -213,10 +214,11 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
skb->pkt_type = PACKET_HOST;
skb->protocol = eth_type_trans(skb, skb->dev);
- u64_stats_update_begin(&p->stats64.syncp);
- p->stats64.rx_packets++;
- p->stats64.rx_bytes += skb->len;
- u64_stats_update_end(&p->stats64.syncp);
+ s = this_cpu_ptr(p->stats64);
+ u64_stats_update_begin(&s->syncp);
+ s->rx_packets++;
+ s->rx_bytes += skb->len;
+ u64_stats_update_end(&s->syncp);
netif_receive_skb(skb);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 7aa0656296c2..306cff229def 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -77,7 +77,7 @@ struct dsa_slave_priv {
struct sk_buff * (*xmit)(struct sk_buff *skb,
struct net_device *dev);
- struct pcpu_sw_netstats stats64;
+ struct pcpu_sw_netstats *stats64;
/* DSA port data, such as switch, port index, etc. */
struct dsa_port *dp;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index e196562035b1..605444ced06c 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -352,12 +352,14 @@ static inline netdev_tx_t dsa_netpoll_send_skb(struct dsa_slave_priv *p,
static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct dsa_slave_priv *p = netdev_priv(dev);
+ struct pcpu_sw_netstats *s;
struct sk_buff *nskb;
- u64_stats_update_begin(&p->stats64.syncp);
- p->stats64.tx_packets++;
- p->stats64.tx_bytes += skb->len;
- u64_stats_update_end(&p->stats64.syncp);
+ s = this_cpu_ptr(p->stats64);
+ u64_stats_update_begin(&s->syncp);
+ s->tx_packets++;
+ s->tx_bytes += skb->len;
+ u64_stats_update_end(&s->syncp);
/* Transmit function may have to reallocate the original SKB,
* in which case it must have freed it. Only free it here on error.
@@ -596,15 +598,26 @@ static void dsa_slave_get_ethtool_stats(struct net_device *dev,
{
struct dsa_slave_priv *p = netdev_priv(dev);
struct dsa_switch *ds = p->dp->ds;
+ struct pcpu_sw_netstats *s;
unsigned int start;
-
- do {
- start = u64_stats_fetch_begin_irq(&p->stats64.syncp);
- data[0] = p->stats64.tx_packets;
- data[1] = p->stats64.tx_bytes;
- data[2] = p->stats64.rx_packets;
- data[3] = p->stats64.rx_bytes;
- } while (u64_stats_fetch_retry_irq(&p->stats64.syncp, start));
+ int i;
+
+ for_each_possible_cpu(i) {
+ u64 tx_packets, tx_bytes, rx_packets, rx_bytes;
+
+ s = per_cpu_ptr(p->stats64, i);
+ do {
+ start = u64_stats_fetch_begin_irq(&s->syncp);
+ tx_packets = s->tx_packets;
+ tx_bytes = s->tx_bytes;
+ rx_packets = s->rx_packets;
+ rx_bytes = s->rx_bytes;
+ } while (u64_stats_fetch_retry_irq(&s->syncp, start));
+ data[0] += tx_packets;
+ data[1] += tx_bytes;
+ data[2] += rx_packets;
+ data[3] += rx_bytes;
+ }
if (ds->ops->get_ethtool_stats)
ds->ops->get_ethtool_stats(ds, p->dp->index, data + 4);
}
@@ -879,16 +892,28 @@ static void dsa_slave_get_stats64(struct net_device *dev,
struct rtnl_link_stats64 *stats)
{
struct dsa_slave_priv *p = netdev_priv(dev);
+ struct pcpu_sw_netstats *s;
unsigned int start;
+ int i;
netdev_stats_to_stats64(stats, &dev->stats);
- do {
- start = u64_stats_fetch_begin_irq(&p->stats64.syncp);
- stats->tx_packets = p->stats64.tx_packets;
- stats->tx_bytes = p->stats64.tx_bytes;
- stats->rx_packets = p->stats64.rx_packets;
- stats->rx_bytes = p->stats64.rx_bytes;
- } while (u64_stats_fetch_retry_irq(&p->stats64.syncp, start));
+ for_each_possible_cpu(i) {
+ u64 tx_packets, tx_bytes, rx_packets, rx_bytes;
+
+ s = per_cpu_ptr(p->stats64, i);
+ do {
+ start = u64_stats_fetch_begin_irq(&s->syncp);
+ tx_packets = s->tx_packets;
+ tx_bytes = s->tx_bytes;
+ rx_packets = s->rx_packets;
+ rx_bytes = s->rx_bytes;
+ } while (u64_stats_fetch_retry_irq(&s->syncp, start));
+
+ stats->tx_packets += tx_packets;
+ stats->tx_bytes += tx_bytes;
+ stats->rx_packets += rx_packets;
+ stats->rx_bytes += rx_bytes;
+ }
}
void dsa_cpu_port_ethtool_init(struct ethtool_ops *ops)
@@ -1202,7 +1227,11 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
slave_dev->vlan_features = master->vlan_features;
p = netdev_priv(slave_dev);
- u64_stats_init(&p->stats64.syncp);
+ p->stats64 = netdev_alloc_pcpu_stats(struct pcpu_sw_netstats);
+ if (!p->stats64) {
+ free_netdev(slave_dev);
+ return -ENOMEM;
+ }
p->dp = &ds->ports[port];
INIT_LIST_HEAD(&p->mall_tc_list);
p->xmit = dst->tag_ops->xmit;
@@ -1217,6 +1246,7 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
netdev_err(master, "error %d registering interface %s\n",
ret, slave_dev->name);
ds->ports[port].netdev = NULL;
+ free_percpu(p->stats64);
free_netdev(slave_dev);
return ret;
}
@@ -1227,6 +1257,7 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
if (ret) {
netdev_err(master, "error %d setting up slave phy\n", ret);
unregister_netdev(slave_dev);
+ free_percpu(p->stats64);
free_netdev(slave_dev);
return ret;
}
@@ -1249,6 +1280,7 @@ void dsa_slave_destroy(struct net_device *slave_dev)
of_phy_deregister_fixed_link(port_dn);
}
unregister_netdev(slave_dev);
+ free_percpu(p->stats64);
free_netdev(slave_dev);
}
--
2.9.3
^ permalink raw reply related
* Re: [PATCH net-next v4 0/9] socket sendmsg MSG_ZEROCOPY
From: David Miller @ 2017-08-04 4:37 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: netdev, linux-api, willemb
In-Reply-To: <20170803202945.70750-1-willemdebruijn.kernel@gmail.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Thu, 3 Aug 2017 16:29:36 -0400
> Introduce zerocopy socket send flag MSG_ZEROCOPY. This extends the
> shared page support (SKBTX_SHARED_FRAG) from sendpage to sendmsg.
> Implement the feature for TCP initially, as large writes benefit
> most.
Looks great, series applied, thanks!
^ permalink raw reply
* Re: [RFC PATCH 4/6] net: sockmap with sk redirect support
From: John Fastabend @ 2017-08-04 4:49 UTC (permalink / raw)
To: Tom Herbert
Cc: David S. Miller, Alexei Starovoitov,
Linux Kernel Network Developers, Daniel Borkmann
In-Reply-To: <CALx6S36fmiyFNTif2B6X91wPXWEb1W3pk5=N0JTMD0T9ZoL5fw@mail.gmail.com>
On 08/03/2017 09:22 PM, Tom Herbert wrote:
> On Thu, Aug 3, 2017 at 4:37 PM, John Fastabend <john.fastabend@gmail.com> wrote:
>> Recently we added a new map type called dev map used to forward XDP
>> packets between ports (6093ec2dc313). This patches introduces a
>> similar notion for sockets.
>>
>> A sockmap allows users to add participating sockets to a map. When
>> sockets are added to the map enough context is stored with the
>> map entry to use the entry with a new helper
>>
>> bpf_sk_redirect_map(map, key, flags)
>>
>> This helper (analogous to bpf_redirect_map in XDP) is given the map
>> and an entry in the map. When called from a sockmap program, discussed
>> below, the skb will be sent on the socket using skb_send_sock().
>>
>> With the above we need a bpf program to call the helper from that will
>> then implement the send logic. The initial site implemented in this
>> series is the recv_sock hook. For this to work we implemented a map
>> attach command to add attributes to a map. In sockmap we add two
>> programs a parse program and a verdict program. The parse program
>> uses strparser to build messages and pass them to the verdict program.
>> The parse program usese normal strparser semantics. The verdict
>> program is of type SOCKET_FILTER.
>>
>> The verdict program returns a verdict BPF_OK, BPF_DROP, BPF_REDIRECT.
>> When BPF_REDIRECT is returned, expected when bpf program uses
>> bpf_sk_redirect_map(), the sockmap logic will consult per cpu variables
>> set by the helper routine and pull the sock entry out of the sock map.
>> This pattern follows the existing redirect logic in cls and xdp
>> programs.
>>
> Hi John,
>
> I'm a bit confused. If the verdict program bpf_mux then? I don't see
> any use of BPF_OK,DROP, or REDIRECT. I assume I'm missing something.
>
> Tom
Ah so what I coded and what I wrote don't align perfectly here. The
verdict program _is_ bpf_mux as you guessed. I should rename the code
to use bpf_verdict (or come up with a better name). Calling it bpf_mux
was a hold out from a very specific example program I wrote up.
Then BPF_OK_DROP and BPF_REDIRECT still need to be included in below.
[...]
>> +
>> +static struct smap_psock *smap_peers_get(struct smap_psock *psock,
>> + struct sk_buff *skb)
>> +{
>> + struct sock *sock;
>> + int rc;
>> +
>> + rc = smap_mux_func(psock, skb);
>> + if (unlikely(rc < 0))
>> + return NULL;
>> +
replacing the above 3 lines with the following should align the commit
message and code,
rc = smap_mux_func(psock, skb);
if (rc != BPF_REDIRECT)
return NULL;
>> + sock = do_sk_redirect_map();
>> + if (unlikely(!sock))
>> + return NULL;
>> +
>> + return smap_psock_sk(sock);
>> +}
>> +
And then in uapi/bpf.h
enum {
BPF_OK_DROP,
BPF_REDIRECT,
}
Thanks,
John
^ permalink raw reply
* Re: [RFC PATCH 4/6] net: sockmap with sk redirect support
From: John Fastabend @ 2017-08-04 4:49 UTC (permalink / raw)
To: Tom Herbert
Cc: David S. Miller, Alexei Starovoitov,
Linux Kernel Network Developers, Daniel Borkmann
In-Reply-To: <CALx6S36fmiyFNTif2B6X91wPXWEb1W3pk5=N0JTMD0T9ZoL5fw@mail.gmail.com>
On 08/03/2017 09:22 PM, Tom Herbert wrote:
> On Thu, Aug 3, 2017 at 4:37 PM, John Fastabend <john.fastabend@gmail.com> wrote:
>> Recently we added a new map type called dev map used to forward XDP
>> packets between ports (6093ec2dc313). This patches introduces a
>> similar notion for sockets.
>>
>> A sockmap allows users to add participating sockets to a map. When
>> sockets are added to the map enough context is stored with the
>> map entry to use the entry with a new helper
>>
>> bpf_sk_redirect_map(map, key, flags)
>>
>> This helper (analogous to bpf_redirect_map in XDP) is given the map
>> and an entry in the map. When called from a sockmap program, discussed
>> below, the skb will be sent on the socket using skb_send_sock().
>>
>> With the above we need a bpf program to call the helper from that will
>> then implement the send logic. The initial site implemented in this
>> series is the recv_sock hook. For this to work we implemented a map
>> attach command to add attributes to a map. In sockmap we add two
>> programs a parse program and a verdict program. The parse program
>> uses strparser to build messages and pass them to the verdict program.
>> The parse program usese normal strparser semantics. The verdict
>> program is of type SOCKET_FILTER.
>>
>> The verdict program returns a verdict BPF_OK, BPF_DROP, BPF_REDIRECT.
>> When BPF_REDIRECT is returned, expected when bpf program uses
>> bpf_sk_redirect_map(), the sockmap logic will consult per cpu variables
>> set by the helper routine and pull the sock entry out of the sock map.
>> This pattern follows the existing redirect logic in cls and xdp
>> programs.
>>
> Hi John,
>
> I'm a bit confused. If the verdict program bpf_mux then? I don't see
> any use of BPF_OK,DROP, or REDIRECT. I assume I'm missing something.
>
> Tom
Ah so what I coded and what I wrote don't align perfectly here. The
verdict program _is_ bpf_mux as you guessed. I should rename the code
to use bpf_verdict (or come up with a better name). Calling it bpf_mux
was a hold out from a very specific example program I wrote up.
Then BPF_OK_DROP and BPF_REDIRECT still need to be included in below.
[...]
>> +
>> +static struct smap_psock *smap_peers_get(struct smap_psock *psock,
>> + struct sk_buff *skb)
>> +{
>> + struct sock *sock;
>> + int rc;
>> +
>> + rc = smap_mux_func(psock, skb);
>> + if (unlikely(rc < 0))
>> + return NULL;
>> +
replacing the above 3 lines with the following should align the commit
message and code,
rc = smap_mux_func(psock, skb);
if (rc != BPF_REDIRECT)
return NULL;
>> + sock = do_sk_redirect_map();
>> + if (unlikely(!sock))
>> + return NULL;
>> +
>> + return smap_psock_sk(sock);
>> +}
>> +
And then in uapi/linux/bpf.h
enum {
BPF_OK_DROP,
BPF_REDIRECT,
}
Thanks,
John
^ permalink raw reply
* [net-next PATCH] net: comment fixes against BPF devmap helper calls
From: John Fastabend @ 2017-08-04 4:58 UTC (permalink / raw)
To: davem; +Cc: netdev, daniel, ast
Update BPF comments to accurately reflect XDP usage.
Fixes: 97f91a7cf04ff ("bpf: add bpf_redirect_map helper routine")
Reported-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/uapi/linux/bpf.h | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 1106a8c..1ae061e 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -345,14 +345,20 @@ enum bpf_attach_type {
* int bpf_redirect(ifindex, flags)
* redirect to another netdev
* @ifindex: ifindex of the net device
- * @flags: bit 0 - if set, redirect to ingress instead of egress
- * other bits - reserved
- * Return: TC_ACT_REDIRECT
- * int bpf_redirect_map(key, map, flags)
+ * @flags:
+ * cls_bpf:
+ * bit 0 - if set, redirect to ingress instead of egress
+ * other bits - reserved
+ * xdp_bpf:
+ * all bits - reserved
+ * Return: cls_bpf: TC_ACT_REDIRECT
+ * xdp_bfp: XDP_REDIRECT
+ * int bpf_redirect_map(map, key, flags)
* redirect to endpoint in map
+ * @map: pointer to dev map
* @key: index in map to lookup
- * @map: fd of map to do lookup in
* @flags: --
+ * Return: XDP_REDIRECT on success or XDP_ABORT on error
*
* u32 bpf_get_route_realm(skb)
* retrieve a dst's tclassid
^ permalink raw reply related
* Re: [PATCH net-next v2 00/13] Change DSA's FDB API and perform switchdev cleanup
From: Jiri Pirko @ 2017-08-04 5:23 UTC (permalink / raw)
To: Arkadi Sharshevsky
Cc: Vivien Didelot, netdev, davem, ivecera, f.fainelli, andrew,
Woojung.Huh, stephen, mlxsw
In-Reply-To: <aa4f3dc5-6693-4de8-5191-f0a939ff8168@mellanox.com>
Fri, Aug 04, 2017 at 12:39:02AM CEST, arkadis@mellanox.com wrote:
>
>[...]
>
>>> Now we have the "offload" read only flag, which is good to inform about
>>> a successfully programmed hardware, but adds another level of complexity
>>> to understand the interaction with the hardware.
>>>
>>> I think iproute2 is getting more and more confusing. From what I
>>> understood, respecting the "self" flag as described is not possible
>>> anymore due to some retro-compatibility reasons.
>>>
>>> Also Linux must use the hardware as an accelerator (so "self" or
>>> "offload" must be the default), and always fall back to software
>>> otherwise, hence "master" do not make sense here.
>>>
>>> What do you think about this synopsis for bridge fdb add?
>>>
>>> # bridge fdb add LLADDR dev DEV [ offload { on | off } ]
>>>
>>> Where offload defaults to "on". This option should also be ported to
>>> other offloaded features like MDB and VLAN. Even though this is a bit
>>> out of scope of this patchset, do you think this is feasible?
>>>
>>
>> I agree completely that currently its confusing. The documentation
>> should be updated for sure. I think that 'self' was primarily introduced
>> (Commit 77162022a) for NIC embedded switches which are used for sriov, in
>> that case the self is related to the internal eswitch, which completely
>> diverge from the software one (clearly not swithcdev).
>>
>> IMHO For switchdev devices 'self' should not be an option at all, or any
>> other arg regarding hardware. Furthermore, the 'offload' flag should be
>> only relevant during the dump as an indication to the user.
>>
>> Unfortunately, the lack of ability of syncing the sw with hw in DSA's
>> case introduces a problem for indicating that the entries are only
>> in hw, I mean marking it only as offloaded is not enough.
>
>Hi,
>
>It seems impossible currently to move the self to be the default, and
>this introduces regression which you don't approve, so it seems few
>options left:
>
>a) Leave two ways to add fdb, through the bridge (by using the master
> flag) which is introduced in this patchset, and by using the self
> which is the legacy way. In this way no regression will be introduced,
> yet, it feels confusing a bit. The benefit is that we (DSA/mlxsw)
> will be synced.
>b) Leave only the self (which means removing patch no 4,5).
I belive that option a) is the correct way to go. Introduction of self
inclusion was a mistake from the very beginning. I think that we should
just move one and correct this mistake.
Vivien, any arguments against a)?
Thanks!
>
>In both cases the switchdev implementation of .ndo_fdb_add() will be
>moved inside DSA in a similar way to the dump because its only used by
>you.
>
>Option b) actually turns this patchset into cosmetic one which does
>only cleanup.
>
>Thanks,
>Arkadi
>
>
>
>
^ permalink raw reply
* Re: [PATCH net-next] net: dsa: User per-cpu 64-bit statistics
From: Eric Dumazet @ 2017-08-04 5:36 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, davem, Andrew Lunn, Vivien Didelot, David S. Miller,
open list
In-Reply-To: <20170804043328.4730-1-f.fainelli@gmail.com>
On Thu, 2017-08-03 at 21:33 -0700, Florian Fainelli wrote:
> During testing with a background iperf pushing 1Gbit/sec worth of
> traffic and having both ifconfig and ethtool collect statistics, we
> could see quite frequent deadlocks. Convert the often accessed DSA slave
> network devices statistics to per-cpu 64-bit statistics to remove these
> deadlocks and provide fast efficient statistics updates.
>
This seems to be a bug fix, it would be nice to get a proper tag like :
Fixes: f613ed665bb3 ("net: dsa: Add support for 64-bit statistics")
Problem here is that if multiple cpus can call dsa_switch_rcv() at the
same time, then u64_stats_update_begin() contract is not respected.
include/linux/u64_stats_sync.h states :
* Usage :
*
* Stats producer (writer) should use following template granted it already got
* an exclusive access to counters (a lock is already taken, or per cpu
* data is used [in a non preemptable context])
*
* spin_lock_bh(...) or other synchronization to get exclusive access
* ...
* u64_stats_update_begin(&stats->syncp);
^ permalink raw reply
* [PATCH] samples/bpf: Fix cross compiler error with bpf sample
From: Joel Fernandes @ 2017-08-04 5:46 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes, Alexei Starovoitov, Daniel Borkmann,
open list:BPF (Safe dynamic programs and tools)
When cross-compiling the bpf sample map_perf_test for aarch64, I find that
__NR_getpgrp is undefined. This causes build errors. Fix it by allowing the
deprecated syscall in the sample.
Signed-off-by: Joel Fernandes <joelaf@google.com>
---
samples/bpf/map_perf_test_user.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/samples/bpf/map_perf_test_user.c b/samples/bpf/map_perf_test_user.c
index 1a8894b5ac51..6e6fc7121640 100644
--- a/samples/bpf/map_perf_test_user.c
+++ b/samples/bpf/map_perf_test_user.c
@@ -8,7 +8,9 @@
#include <sched.h>
#include <stdio.h>
#include <sys/types.h>
+#define __ARCH_WANT_SYSCALL_DEPRECATED
#include <asm/unistd.h>
+#undef __ARCH_WANT_SYSCALL_DEPRECATED
#include <unistd.h>
#include <assert.h>
#include <sys/wait.h>
--
2.14.0.rc1.383.gd1ce394fe2-goog
^ permalink raw reply related
* RE: [PATCH net 3/3] tcp: fix xmit timer to only be reset if data ACKed/SACKed
From: maowenan @ 2017-08-04 7:12 UTC (permalink / raw)
To: maowenan, Neal Cardwell, David Miller
Cc: netdev@vger.kernel.org, Yuchung Cheng, Nandita Dukkipati
In-Reply-To: <20170801025814.31206-4-ncardwell@google.com>
> -----Original Message-----
> From: maowenan
> Sent: Tuesday, August 01, 2017 8:20 PM
> To: 'Neal Cardwell'; David Miller
> Cc: netdev@vger.kernel.org; Yuchung Cheng; Nandita Dukkipati
> Subject: RE: [PATCH net 3/3] tcp: fix xmit timer to only be reset if data
> ACKed/SACKed
>
>
>
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org
> > [mailto:netdev-owner@vger.kernel.org]
> > On Behalf Of Neal Cardwell
> > Sent: Tuesday, August 01, 2017 10:58 AM
> > To: David Miller
> > Cc: netdev@vger.kernel.org; Neal Cardwell; Yuchung Cheng; Nandita
> > Dukkipati
> > Subject: [PATCH net 3/3] tcp: fix xmit timer to only be reset if data
> > ACKed/SACKed
> >
> > Fix a TCP loss recovery performance bug raised recently on the netdev
> > list, in two threads:
> >
> > (i) July 26, 2017: netdev thread "TCP fast retransmit issues"
> > (ii) July 26, 2017: netdev thread:
> > "[PATCH V2 net-next] TLP: Don't reschedule PTO when there's one
> > outstanding TLP retransmission"
> >
> > The basic problem is that incoming TCP packets that did not indicate
> > forward progress could cause the xmit timer (TLP or RTO) to be rearmed
> > and pushed back in time. In certain corner cases this could result in
> > the following problems noted in these threads:
> >
> > - Repeated ACKs coming in with bogus SACKs corrupted by middleboxes
> > could cause TCP to repeatedly schedule TLPs forever. We kept
> > sending TLPs after every ~200ms, which elicited bogus SACKs, which
> > caused more TLPs, ad infinitum; we never fired an RTO to fill in
> > the holes.
> >
> > - Incoming data segments could, in some cases, cause us to reschedule
> > our RTO or TLP timer further out in time, for no good reason. This
> > could cause repeated inbound data to result in stalls in outbound
> > data, in the presence of packet loss.
> >
> > This commit fixes these bugs by changing the TLP and RTO ACK processing to:
> >
> > (a) Only reschedule the xmit timer once per ACK.
> >
> > (b) Only reschedule the xmit timer if tcp_clean_rtx_queue() deems the
> > ACK indicates sufficient forward progress (a packet was
> > cumulatively ACKed, or we got a SACK for a packet that was sent
> > before the most recent retransmit of the write queue head).
> >
> > This brings us back into closer compliance with the RFCs, since, as
> > the comment for tcp_rearm_rto() notes, we should only restart the RTO
> > timer after forward progress on the connection. Previously we were
> > restarting the xmit timer even in these cases where there was no forward
> progress.
> >
> > As a side benefit, this commit simplifies and speeds up the TCP timer
> > arming logic. We had been calling inet_csk_reset_xmit_timer() three
> > times on normal ACKs that cumulatively acknowledged some data:
> >
> > 1) Once near the top of tcp_ack() to switch from TLP timer to RTO:
> > if (icsk->icsk_pending == ICSK_TIME_LOSS_PROBE)
> > tcp_rearm_rto(sk);
> >
> > 2) Once in tcp_clean_rtx_queue(), to update the RTO:
> > if (flag & FLAG_ACKED) {
> > tcp_rearm_rto(sk);
> >
> > 3) Once in tcp_ack() after tcp_fastretrans_alert() to switch from RTO
> > to TLP:
> > if (icsk->icsk_pending == ICSK_TIME_RETRANS)
> > tcp_schedule_loss_probe(sk);
> >
> > This commit, by only rescheduling the xmit timer once per ACK,
> > simplifies the code and reduces CPU overhead.
> >
> > This commit was tested in an A/B test with Google web server traffic.
> > SNMP stats and request latency metrics were within noise levels,
> > substantiating that for normal web traffic patterns this is a rare
> > issue. This commit was also tested with packetdrill tests to verify
> > that it fixes the timer behavior in the corner cases discussed in the netdev
> threads mentioned above.
> >
> > This patch is a bug fix patch intended to be queued for -stable relases.
> >
> > Fixes: 6ba8a3b19e76 ("tcp: Tail loss probe (TLP)")
> > Reported-by: Klavs Klavsen <kl@vsen.dk>
> > Reported-by: Mao Wenan <maowenan@huawei.com>
> > Signed-off-by: Neal Cardwell <ncardwell@google.com>
> > Signed-off-by: Yuchung Cheng <ycheng@google.com>
> > Signed-off-by: Nandita Dukkipati <nanditad@google.com>
> > ---
> > net/ipv4/tcp_input.c | 25 ++++++++++++++++---------
> > net/ipv4/tcp_output.c | 9 ---------
> > 2 files changed, 16 insertions(+), 18 deletions(-)
> >
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index
> > 345febf0a46e..3e777cfbba56 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -107,6 +107,7 @@ int sysctl_tcp_invalid_ratelimit __read_mostly =
> HZ/2;
> > #define FLAG_ORIG_SACK_ACKED 0x200 /* Never retransmitted data are
> > (s)acked */
> > #define FLAG_SND_UNA_ADVANCED 0x400 /* Snd_una was changed (!=
> > FLAG_DATA_ACKED) */
> > #define FLAG_DSACKING_ACK 0x800 /* SACK blocks contained D-SACK info
> > */
> > +#define FLAG_SET_XMIT_TIMER 0x1000 /* Set TLP or RTO timer */
> > #define FLAG_SACK_RENEGING 0x2000 /* snd_una advanced to a
> sacked
> > seq */
> > #define FLAG_UPDATE_TS_RECENT 0x4000 /* tcp_replace_ts_recent() */
> > #define FLAG_NO_CHALLENGE_ACK 0x8000 /* do not call
> > tcp_send_challenge_ack() */
> > @@ -3016,6 +3017,13 @@ void tcp_rearm_rto(struct sock *sk)
> > }
> > }
> >
> > +/* Try to schedule a loss probe; if that doesn't work, then schedule
> > +an RTO. */ static void tcp_set_xmit_timer(struct sock *sk) {
> > + if (!tcp_schedule_loss_probe(sk))
> > + tcp_rearm_rto(sk);
> > +}
> > +
> > /* If we get here, the whole TSO packet has not been acked. */
> > static u32 tcp_tso_acked(struct sock *sk, struct sk_buff *skb) { @@
> > -3177,7 +3185,7 @@ static int tcp_clean_rtx_queue(struct sock *sk, int
> prior_fackets,
> > ca_rtt_us, sack->rate);
> >
> > if (flag & FLAG_ACKED) {
> > - tcp_rearm_rto(sk);
> > + flag |= FLAG_SET_XMIT_TIMER; /* set TLP or RTO timer */
> > if (unlikely(icsk->icsk_mtup.probe_size &&
> > !after(tp->mtu_probe.probe_seq_end, tp->snd_una))) {
> > tcp_mtup_probe_success(sk);
> > @@ -3205,7 +3213,7 @@ static int tcp_clean_rtx_queue(struct sock *sk,
> > int prior_fackets,
> > * after when the head was last (re)transmitted. Otherwise the
> > * timeout may continue to extend in loss recovery.
> > */
> > - tcp_rearm_rto(sk);
> > + flag |= FLAG_SET_XMIT_TIMER; /* set TLP or RTO timer */
> > }
> >
> > if (icsk->icsk_ca_ops->pkts_acked) { @@ -3577,9 +3585,6 @@ static
> > int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
> > if (after(ack, tp->snd_nxt))
> > goto invalid_ack;
> >
> > - if (icsk->icsk_pending == ICSK_TIME_LOSS_PROBE)
> > - tcp_rearm_rto(sk);
> > -
> > if (after(ack, prior_snd_una)) {
> > flag |= FLAG_SND_UNA_ADVANCED;
> > icsk->icsk_retransmits = 0;
> > @@ -3644,18 +3649,20 @@ static int tcp_ack(struct sock *sk, const
> > struct sk_buff *skb, int flag)
> > flag |= tcp_clean_rtx_queue(sk, prior_fackets, prior_snd_una, &acked,
> > &sack_state);
> >
> > + if (tp->tlp_high_seq)
> > + tcp_process_tlp_ack(sk, ack, flag);
> > + /* If needed, reset TLP/RTO timer; RACK may later override this. */
> [Mao Wenan] I have question about RACK, if there is no RACK feature in lower
> version, who can clear this flag:FLAG_SET_XMIT_TIMER?
>
> > + if (flag & FLAG_SET_XMIT_TIMER)
> > + tcp_set_xmit_timer(sk);
> > +
> > if (tcp_ack_is_dubious(sk, flag)) {
> > is_dupack = !(flag & (FLAG_SND_UNA_ADVANCED |
> FLAG_NOT_DUP));
> > tcp_fastretrans_alert(sk, acked, is_dupack, &flag, &rexmit);
> > }
> > - if (tp->tlp_high_seq)
> > - tcp_process_tlp_ack(sk, ack, flag);
> >
> > if ((flag & FLAG_FORWARD_PROGRESS) || !(flag & FLAG_NOT_DUP))
> > sk_dst_confirm(sk);
> >
> > - if (icsk->icsk_pending == ICSK_TIME_RETRANS)
> > - tcp_schedule_loss_probe(sk);
> > delivered = tp->delivered - delivered; /* freshly ACKed or SACKed */
> > lost = tp->lost - lost; /* freshly marked lost */
> > tcp_rate_gen(sk, delivered, lost, sack_state.rate); diff --git
> > a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index
> > 0ae6b5d176c0..c99cba897b9c 100644
> > --- a/net/ipv4/tcp_output.c
> > +++ b/net/ipv4/tcp_output.c
> > @@ -2380,21 +2380,12 @@ bool tcp_schedule_loss_probe(struct sock *sk)
> > u32 rtt = usecs_to_jiffies(tp->srtt_us >> 3);
> > u32 timeout, rto_delta_us;
> >
> > - /* No consecutive loss probes. */
> > - if (WARN_ON(icsk->icsk_pending == ICSK_TIME_LOSS_PROBE)) {
> > - tcp_rearm_rto(sk);
> > - return false;
> > - }
[Mao Wenan] I'm sorry I can't get why you delete this and below "if" branch?
> > /* Don't do any loss probe on a Fast Open connection before 3WHS
> > * finishes.
> > */
> > if (tp->fastopen_rsk)
> > return false;
> >
> > - /* TLP is only scheduled when next timer event is RTO. */
> > - if (icsk->icsk_pending != ICSK_TIME_RETRANS)
> > - return false;
> > -
> > /* Schedule a loss probe in 2*RTT for SACK capable connections
> > * in Open state, that are either limited by cwnd or application.
> > */
> > --
> > 2.14.0.rc0.400.g1c36432dff-goog
^ permalink raw reply
* RE: [PATCH net 3/3] tcp: fix xmit timer to only be reset if data ACKed/SACKed
From: maowenan @ 2017-08-04 7:33 UTC (permalink / raw)
To: maowenan, Neal Cardwell, David Miller
Cc: netdev@vger.kernel.org, Yuchung Cheng, Nandita Dukkipati
In-Reply-To: <20170801025814.31206-4-ncardwell@google.com>
> -----Original Message-----
> From: maowenan
> Sent: Tuesday, August 01, 2017 8:20 PM
> To: 'Neal Cardwell'; David Miller
> Cc: netdev@vger.kernel.org; Yuchung Cheng; Nandita Dukkipati
> Subject: RE: [PATCH net 3/3] tcp: fix xmit timer to only be reset if data
> ACKed/SACKed
>
>
>
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org
> > [mailto:netdev-owner@vger.kernel.org]
> > On Behalf Of Neal Cardwell
> > Sent: Tuesday, August 01, 2017 10:58 AM
> > To: David Miller
> > Cc: netdev@vger.kernel.org; Neal Cardwell; Yuchung Cheng; Nandita
> > Dukkipati
> > Subject: [PATCH net 3/3] tcp: fix xmit timer to only be reset if data
> > ACKed/SACKed
> >
> > Fix a TCP loss recovery performance bug raised recently on the netdev
> > list, in two threads:
> >
> > (i) July 26, 2017: netdev thread "TCP fast retransmit issues"
> > (ii) July 26, 2017: netdev thread:
> > "[PATCH V2 net-next] TLP: Don't reschedule PTO when there's one
> > outstanding TLP retransmission"
> >
> > The basic problem is that incoming TCP packets that did not indicate
> > forward progress could cause the xmit timer (TLP or RTO) to be rearmed
> > and pushed back in time. In certain corner cases this could result in
> > the following problems noted in these threads:
> >
> > - Repeated ACKs coming in with bogus SACKs corrupted by middleboxes
> > could cause TCP to repeatedly schedule TLPs forever. We kept
> > sending TLPs after every ~200ms, which elicited bogus SACKs, which
> > caused more TLPs, ad infinitum; we never fired an RTO to fill in
> > the holes.
> >
> > - Incoming data segments could, in some cases, cause us to reschedule
> > our RTO or TLP timer further out in time, for no good reason. This
> > could cause repeated inbound data to result in stalls in outbound
> > data, in the presence of packet loss.
> >
> > This commit fixes these bugs by changing the TLP and RTO ACK processing to:
> >
> > (a) Only reschedule the xmit timer once per ACK.
> >
> > (b) Only reschedule the xmit timer if tcp_clean_rtx_queue() deems the
> > ACK indicates sufficient forward progress (a packet was
> > cumulatively ACKed, or we got a SACK for a packet that was sent
> > before the most recent retransmit of the write queue head).
> >
> > This brings us back into closer compliance with the RFCs, since, as
> > the comment for tcp_rearm_rto() notes, we should only restart the RTO
> > timer after forward progress on the connection. Previously we were
> > restarting the xmit timer even in these cases where there was no forward
> progress.
> >
> > As a side benefit, this commit simplifies and speeds up the TCP timer
> > arming logic. We had been calling inet_csk_reset_xmit_timer() three
> > times on normal ACKs that cumulatively acknowledged some data:
> >
> > 1) Once near the top of tcp_ack() to switch from TLP timer to RTO:
> > if (icsk->icsk_pending == ICSK_TIME_LOSS_PROBE)
> > tcp_rearm_rto(sk);
> >
> > 2) Once in tcp_clean_rtx_queue(), to update the RTO:
> > if (flag & FLAG_ACKED) {
> > tcp_rearm_rto(sk);
> >
> > 3) Once in tcp_ack() after tcp_fastretrans_alert() to switch from RTO
> > to TLP:
> > if (icsk->icsk_pending == ICSK_TIME_RETRANS)
> > tcp_schedule_loss_probe(sk);
> >
> > This commit, by only rescheduling the xmit timer once per ACK,
> > simplifies the code and reduces CPU overhead.
> >
> > This commit was tested in an A/B test with Google web server traffic.
> > SNMP stats and request latency metrics were within noise levels,
> > substantiating that for normal web traffic patterns this is a rare
> > issue. This commit was also tested with packetdrill tests to verify
> > that it fixes the timer behavior in the corner cases discussed in the netdev
> threads mentioned above.
> >
> > This patch is a bug fix patch intended to be queued for -stable relases.
> >
> > Fixes: 6ba8a3b19e76 ("tcp: Tail loss probe (TLP)")
> > Reported-by: Klavs Klavsen <kl@vsen.dk>
> > Reported-by: Mao Wenan <maowenan@huawei.com>
> > Signed-off-by: Neal Cardwell <ncardwell@google.com>
> > Signed-off-by: Yuchung Cheng <ycheng@google.com>
> > Signed-off-by: Nandita Dukkipati <nanditad@google.com>
> > ---
> > net/ipv4/tcp_input.c | 25 ++++++++++++++++---------
> > net/ipv4/tcp_output.c | 9 ---------
> > 2 files changed, 16 insertions(+), 18 deletions(-)
> >
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index
> > 345febf0a46e..3e777cfbba56 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -107,6 +107,7 @@ int sysctl_tcp_invalid_ratelimit __read_mostly =
> HZ/2;
> > #define FLAG_ORIG_SACK_ACKED 0x200 /* Never retransmitted data are
> > (s)acked */
> > #define FLAG_SND_UNA_ADVANCED 0x400 /* Snd_una was changed (!=
> > FLAG_DATA_ACKED) */
> > #define FLAG_DSACKING_ACK 0x800 /* SACK blocks contained D-SACK info
> > */
> > +#define FLAG_SET_XMIT_TIMER 0x1000 /* Set TLP or RTO timer */
> > #define FLAG_SACK_RENEGING 0x2000 /* snd_una advanced to a
> sacked
> > seq */
> > #define FLAG_UPDATE_TS_RECENT 0x4000 /* tcp_replace_ts_recent() */
> > #define FLAG_NO_CHALLENGE_ACK 0x8000 /* do not call
> > tcp_send_challenge_ack() */
> > @@ -3016,6 +3017,13 @@ void tcp_rearm_rto(struct sock *sk)
> > }
> > }
> >
> > +/* Try to schedule a loss probe; if that doesn't work, then schedule
> > +an RTO. */ static void tcp_set_xmit_timer(struct sock *sk) {
> > + if (!tcp_schedule_loss_probe(sk))
> > + tcp_rearm_rto(sk);
> > +}
> > +
> > /* If we get here, the whole TSO packet has not been acked. */
> > static u32 tcp_tso_acked(struct sock *sk, struct sk_buff *skb) { @@
> > -3177,7 +3185,7 @@ static int tcp_clean_rtx_queue(struct sock *sk, int
> prior_fackets,
> > ca_rtt_us, sack->rate);
> >
> > if (flag & FLAG_ACKED) {
> > - tcp_rearm_rto(sk);
> > + flag |= FLAG_SET_XMIT_TIMER; /* set TLP or RTO timer */
> > if (unlikely(icsk->icsk_mtup.probe_size &&
> > !after(tp->mtu_probe.probe_seq_end, tp->snd_una))) {
> > tcp_mtup_probe_success(sk);
> > @@ -3205,7 +3213,7 @@ static int tcp_clean_rtx_queue(struct sock *sk,
> > int prior_fackets,
> > * after when the head was last (re)transmitted. Otherwise the
> > * timeout may continue to extend in loss recovery.
> > */
> > - tcp_rearm_rto(sk);
> > + flag |= FLAG_SET_XMIT_TIMER; /* set TLP or RTO timer */
> > }
> >
> > if (icsk->icsk_ca_ops->pkts_acked) { @@ -3577,9 +3585,6 @@ static
> > int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
> > if (after(ack, tp->snd_nxt))
> > goto invalid_ack;
> >
> > - if (icsk->icsk_pending == ICSK_TIME_LOSS_PROBE)
> > - tcp_rearm_rto(sk);
> > -
> > if (after(ack, prior_snd_una)) {
> > flag |= FLAG_SND_UNA_ADVANCED;
> > icsk->icsk_retransmits = 0;
> > @@ -3644,18 +3649,20 @@ static int tcp_ack(struct sock *sk, const
> > struct sk_buff *skb, int flag)
> > flag |= tcp_clean_rtx_queue(sk, prior_fackets, prior_snd_una, &acked,
> > &sack_state);
> >
> > + if (tp->tlp_high_seq)
> > + tcp_process_tlp_ack(sk, ack, flag);
> > + /* If needed, reset TLP/RTO timer; RACK may later override this. */
> [Mao Wenan] I have question about RACK, if there is no RACK feature in lower
> version, who can clear this flag:FLAG_SET_XMIT_TIMER?
>
> > + if (flag & FLAG_SET_XMIT_TIMER)
> > + tcp_set_xmit_timer(sk);
> > +
> > if (tcp_ack_is_dubious(sk, flag)) {
> > is_dupack = !(flag & (FLAG_SND_UNA_ADVANCED |
> FLAG_NOT_DUP));
> > tcp_fastretrans_alert(sk, acked, is_dupack, &flag, &rexmit);
> > }
> > - if (tp->tlp_high_seq)
> > - tcp_process_tlp_ack(sk, ack, flag);
> >
> > if ((flag & FLAG_FORWARD_PROGRESS) || !(flag & FLAG_NOT_DUP))
> > sk_dst_confirm(sk);
> >
> > - if (icsk->icsk_pending == ICSK_TIME_RETRANS)
> > - tcp_schedule_loss_probe(sk);
> > delivered = tp->delivered - delivered; /* freshly ACKed or SACKed */
> > lost = tp->lost - lost; /* freshly marked lost */
> > tcp_rate_gen(sk, delivered, lost, sack_state.rate); diff --git
> > a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index
> > 0ae6b5d176c0..c99cba897b9c 100644
> > --- a/net/ipv4/tcp_output.c
> > +++ b/net/ipv4/tcp_output.c
> > @@ -2380,21 +2380,12 @@ bool tcp_schedule_loss_probe(struct sock *sk)
> > u32 rtt = usecs_to_jiffies(tp->srtt_us >> 3);
> > u32 timeout, rto_delta_us;
> >
> > - /* No consecutive loss probes. */
> > - if (WARN_ON(icsk->icsk_pending == ICSK_TIME_LOSS_PROBE)) {
> > - tcp_rearm_rto(sk);
> > - return false;
> > - }
[Mao Wenan]Follow previous mail, in lower version such as 3.10, I found
there are many timer type, e.g:ICSK_TIME_EARLY_RETRANS, RTO,PTO
are used. I'm not sure there exist some unknown problem if we don't check
isck_pending here and below if branch? And could you please post one lower
version patch such as 3.10? Thanks a lot.
#define ICSK_TIME_RETRANS 1 /* Retransmit timer */
#define ICSK_TIME_DACK 2 /* Delayed ack timer */
#define ICSK_TIME_PROBE0 3 /* Zero window probe timer */
#define ICSK_TIME_EARLY_RETRANS 4 /* Early retransmit timer */
#define ICSK_TIME_LOSS_PROBE 5 /* Tail loss probe timer */
> > /* Don't do any loss probe on a Fast Open connection before 3WHS
> > * finishes.
> > */
> > if (tp->fastopen_rsk)
> > return false;
> >
> > - /* TLP is only scheduled when next timer event is RTO. */
> > - if (icsk->icsk_pending != ICSK_TIME_RETRANS)
> > - return false;
> > -
> > /* Schedule a loss probe in 2*RTT for SACK capable connections
> > * in Open state, that are either limited by cwnd or application.
> > */
> > --
> > 2.14.0.rc0.400.g1c36432dff-goog
^ permalink raw reply
* Re: [PATCH v2 05/11] mtd: cfi: reduce stack size with KASAN
From: Boris Brezillon @ 2017-08-04 7:42 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Andrew Morton, kasan-dev, Dmitry Vyukov, Alexander Potapenko,
Andrey Ryabinin, netdev, linux-kernel, Arend van Spriel,
David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
Cyrille Pitchen, linux-mtd
In-Reply-To: <20170614211556.2062728-6-arnd@arndb.de>
On Wed, 14 Jun 2017 23:15:40 +0200
Arnd Bergmann <arnd@arndb.de> wrote:
> When CONFIG_KASAN is used, we consume a lot of extra stack space:
>
> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'do_write_buffer':
> drivers/mtd/chips/cfi_cmdset_0020.c:603:1: error: the frame size of 2184 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_erase_varsize':
> drivers/mtd/chips/cfi_cmdset_0020.c:972:1: error: the frame size of 1936 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> drivers/mtd/chips/cfi_cmdset_0001.c: In function 'do_write_buffer':
> drivers/mtd/chips/cfi_cmdset_0001.c:1841:1: error: the frame size of 1776 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
>
> This marks some functions as noinline_if_stackbloat to keep reduce the
> overall stack size.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/mtd/chips/cfi_cmdset_0020.c | 8 ++++----
> include/linux/mtd/map.h | 8 ++++----
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c b/drivers/mtd/chips/cfi_cmdset_0020.c
> index 7d342965f392..5eee5e883f55 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0020.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0020.c
> @@ -244,7 +244,7 @@ static struct mtd_info *cfi_staa_setup(struct map_info *map)
> }
>
>
> -static inline int do_read_onechip(struct map_info *map, struct flchip *chip, loff_t adr, size_t len, u_char *buf)
> +static noinline_if_stackbloat int do_read_onechip(struct map_info *map, struct flchip *chip, loff_t adr, size_t len, u_char *buf)
Why do we even need to mark those functions inline in the first place?
Isn't the compiler smart enough to decide when it should inline things?
> {
> map_word status, status_OK;
> unsigned long timeo;
> @@ -728,7 +728,7 @@ cfi_staa_writev(struct mtd_info *mtd, const struct kvec *vecs,
> }
>
>
> -static inline int do_erase_oneblock(struct map_info *map, struct flchip *chip, unsigned long adr)
> +static noinline_if_stackbloat int do_erase_oneblock(struct map_info *map, struct flchip *chip, unsigned long adr)
> {
> struct cfi_private *cfi = map->fldrv_priv;
> map_word status, status_OK;
> @@ -1029,7 +1029,7 @@ static void cfi_staa_sync (struct mtd_info *mtd)
> }
> }
>
> -static inline int do_lock_oneblock(struct map_info *map, struct flchip *chip, unsigned long adr)
> +static noinline_if_stackbloat int do_lock_oneblock(struct map_info *map, struct flchip *chip, unsigned long adr)
> {
> struct cfi_private *cfi = map->fldrv_priv;
> map_word status, status_OK;
> @@ -1175,7 +1175,7 @@ static int cfi_staa_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> }
> return 0;
> }
> -static inline int do_unlock_oneblock(struct map_info *map, struct flchip *chip, unsigned long adr)
> +static noinline_if_stackbloat int do_unlock_oneblock(struct map_info *map, struct flchip *chip, unsigned long adr)
> {
> struct cfi_private *cfi = map->fldrv_priv;
> map_word status, status_OK;
> diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
> index 3aa56e3104bb..29db74314db8 100644
> --- a/include/linux/mtd/map.h
> +++ b/include/linux/mtd/map.h
> @@ -316,7 +316,7 @@ static inline map_word map_word_or(struct map_info *map, map_word val1, map_word
> return r;
> }
>
> -static inline int map_word_andequal(struct map_info *map, map_word val1, map_word val2, map_word val3)
> +static noinline_if_stackbloat int map_word_andequal(struct map_info *map, map_word val1, map_word val2, map_word val3)
It's indeed needed for those defined in a header.
> {
> int i;
>
> @@ -328,7 +328,7 @@ static inline int map_word_andequal(struct map_info *map, map_word val1, map_wor
> return 1;
> }
>
> -static inline int map_word_bitsset(struct map_info *map, map_word val1, map_word val2)
> +static noinline_if_stackbloat int map_word_bitsset(struct map_info *map, map_word val1, map_word val2)
> {
> int i;
>
> @@ -362,7 +362,7 @@ static inline map_word map_word_load(struct map_info *map, const void *ptr)
> return r;
> }
>
> -static inline map_word map_word_load_partial(struct map_info *map, map_word orig, const unsigned char *buf, int start, int len)
> +static noinline_if_stackbloat map_word map_word_load_partial(struct map_info *map, map_word orig, const unsigned char *buf, int start, int len)
> {
> int i;
>
> @@ -392,7 +392,7 @@ static inline map_word map_word_load_partial(struct map_info *map, map_word orig
> #define MAP_FF_LIMIT 8
> #endif
>
> -static inline map_word map_word_ff(struct map_info *map)
> +static noinline_if_stackbloat map_word map_word_ff(struct map_info *map)
> {
> map_word r;
> int i;
^ permalink raw reply
* [PATCH] net: phy: marvell: logical vs bitwise OR typo
From: Dan Carpenter @ 2017-08-04 8:17 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, netdev, kernel-janitors
This was supposed to be a bitwise OR but there is a || vs | typo.
Fixes: 864dc729d528 ("net: phy: marvell: Refactor m88e1121 RGMII delay configuration")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 361fe9927ef2..15cbcdba618a 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -83,7 +83,7 @@
#define MII_88E1121_PHY_MSCR_REG 21
#define MII_88E1121_PHY_MSCR_RX_DELAY BIT(5)
#define MII_88E1121_PHY_MSCR_TX_DELAY BIT(4)
-#define MII_88E1121_PHY_MSCR_DELAY_MASK (~(BIT(5) || BIT(4)))
+#define MII_88E1121_PHY_MSCR_DELAY_MASK (~(BIT(5) | BIT(4)))
#define MII_88E1121_MISC_TEST 0x1a
#define MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK 0x1f00
^ permalink raw reply related
* Re: [patch net-next 00/21] mlxsw: Support for IPv6 UC router
From: Ido Schimmel @ 2017-08-04 8:50 UTC (permalink / raw)
To: David Ahern
Cc: David Miller, jiri, netdev, mlxsw, roopa, nikolay, kafai, hannes,
yoshfuji, edumazet, yanhaishuang
In-Reply-To: <172d26bc-cb59-c3c7-7ca4-1308f4853e8c@gmail.com>
On Thu, Aug 03, 2017 at 04:54:35PM -0600, David Ahern wrote:
> On 8/3/17 4:41 PM, David Miller wrote:
> > But unlike the percpu flag, don't we want to somehow propagate offload
> > state to the user?
>
> It's a per nexthop flag. For IPv4 it is tracked in fib_nh.nh_flags.
> Perhaps it is time for rt6_info to have nh_flags as well.
I saw your original mail last night, but was too tired to comment.
I'm looking into reflecting the nexthop status to listeners like what we
did for IPv4 with NH_{ADD,DEL} and can probably incorporate your
suggestion.
Anyway, will make sure RTF_OFFLOAD is removed before the merge window
opens.
Thanks for reviewing David.
^ permalink raw reply
* Re: [PATCH v2 05/11] mtd: cfi: reduce stack size with KASAN
From: Arnd Bergmann @ 2017-08-04 9:09 UTC (permalink / raw)
To: Boris Brezillon
Cc: Andrew Morton, kasan-dev, Dmitry Vyukov, Alexander Potapenko,
Andrey Ryabinin, Networking, Linux Kernel Mailing List,
Arend van Spriel, David Woodhouse, Brian Norris, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, linux-mtd
In-Reply-To: <20170804094254.369140c0@bbrezillon>
On Fri, Aug 4, 2017 at 9:42 AM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Wed, 14 Jun 2017 23:15:40 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
>
>> When CONFIG_KASAN is used, we consume a lot of extra stack space:
>>
>> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'do_write_buffer':
>> drivers/mtd/chips/cfi_cmdset_0020.c:603:1: error: the frame size of 2184 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
>> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_erase_varsize':
>> drivers/mtd/chips/cfi_cmdset_0020.c:972:1: error: the frame size of 1936 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
>> drivers/mtd/chips/cfi_cmdset_0001.c: In function 'do_write_buffer':
>> drivers/mtd/chips/cfi_cmdset_0001.c:1841:1: error: the frame size of 1776 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
>>
>> This marks some functions as noinline_if_stackbloat to keep reduce the
>> overall stack size.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>> drivers/mtd/chips/cfi_cmdset_0020.c | 8 ++++----
>> include/linux/mtd/map.h | 8 ++++----
>> 2 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c b/drivers/mtd/chips/cfi_cmdset_0020.c
>> index 7d342965f392..5eee5e883f55 100644
>> --- a/drivers/mtd/chips/cfi_cmdset_0020.c
>> +++ b/drivers/mtd/chips/cfi_cmdset_0020.c
>> @@ -244,7 +244,7 @@ static struct mtd_info *cfi_staa_setup(struct map_info *map)
>> }
>>
>>
>> -static inline int do_read_onechip(struct map_info *map, struct flchip *chip, loff_t adr, size_t len, u_char *buf)
>> +static noinline_if_stackbloat int do_read_onechip(struct map_info *map, struct flchip *chip, loff_t adr, size_t len, u_char *buf)
>
> Why do we even need to mark those functions inline in the first place?
> Isn't the compiler smart enough to decide when it should inline things?
I'm pretty sure it doesn't need the 'inline' keywork. I see that the code was
addedlike this in linux-2.4.0-test3pre3 along with the rest of the mtd layer,
so it has always been 'inline' and nobody ever bothered to remove that
during later cleanups.
We could probably just mark this function as 'noinline' here and never
inline it,
but I would rather not add more than one variant of noinline_if_stackbloat:
almost all other users of noinline_if_stackbloat are for functions that have
to be inline in normal builds, so it is defined as being either
'inline' or 'noinline'
depending on whether KASAN is active.
>> diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
>> index 3aa56e3104bb..29db74314db8 100644
>> --- a/include/linux/mtd/map.h
>> +++ b/include/linux/mtd/map.h
>> @@ -316,7 +316,7 @@ static inline map_word map_word_or(struct map_info *map, map_word val1, map_word
>> return r;
>> }
>>
>> -static inline int map_word_andequal(struct map_info *map, map_word val1, map_word val2, map_word val3)
>> +static noinline_if_stackbloat int map_word_andequal(struct map_info *map, map_word val1, map_word val2, map_word val3)
>
> It's indeed needed for those defined in a header.
Right.
Arnd
^ permalink raw reply
* [PATCH net-next] net: hns: Fix for __udivdi3 compiler error
From: Yunsheng Lin @ 2017-08-04 9:24 UTC (permalink / raw)
To: davem
Cc: huangdaode, xuwei5, liguozhu, Yisen.Zhuang, gabriele.paoloni,
john.garry, linuxarm, yisen.zhuang, salil.mehta, lipeng321,
tremyfr, netdev, linux-kernel
This patch fixes the __udivdi3 undefined error reported by
test robot.
Fixes: b8c17f708831 ("net: hns: Add self-adaptive interrupt coalesce support in hns driver")
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
drivers/net/ethernet/hisilicon/hns/hns_enet.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index 832f277..3652063 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -847,7 +847,8 @@ static void hns_update_rx_rate(struct hnae_ring *ring)
total_bytes = ring->stats.rx_bytes - ring->coal_last_rx_bytes;
time_passed_ms = jiffies_to_msecs(jiffies - ring->coal_last_jiffies);
- ring->coal_rx_rate = (total_bytes / time_passed_ms) >> 10;
+ do_div(total_bytes, time_passed_ms);
+ ring->coal_rx_rate = total_bytes >> 10;
ring->coal_last_rx_bytes = ring->stats.rx_bytes;
ring->coal_last_jiffies = jiffies;
--
1.9.1
^ permalink raw reply related
* [PATCH] iproute2/misc: do not mix CFLAGS with LDFLAGS
From: Marcus Meissner @ 2017-08-04 9:54 UTC (permalink / raw)
To: netdev; +Cc: Marcus Meissner
during linking, do not use CFLAGS. This avoid clashes when doing PIE builds.
---
misc/Makefile | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/misc/Makefile b/misc/Makefile
index 72807678..1d86c44d 100644
--- a/misc/Makefile
+++ b/misc/Makefile
@@ -23,17 +23,17 @@ all: $(TARGETS)
ss: $(SSOBJ)
$(QUIET_LINK)$(CC) $^ $(LDFLAGS) $(LDLIBS) -o $@
-nstat: nstat.c
- $(QUIET_CC)$(CC) $(CFLAGS) $(LDFLAGS) -o nstat nstat.c $(LIBNETLINK) -lm
+nstat: nstat.o
+ $(QUIET_CC)$(CC) $(LDFLAGS) -o nstat nstat.o $(LIBNETLINK) -lm
-ifstat: ifstat.c
- $(QUIET_CC)$(CC) $(CFLAGS) $(LDFLAGS) -o ifstat ifstat.c $(LIBNETLINK) -lm
+ifstat: ifstat.o
+ $(QUIET_CC)$(CC) $(LDFLAGS) -o ifstat ifstat.o $(LIBNETLINK) -lm
-rtacct: rtacct.c
- $(QUIET_CC)$(CC) $(CFLAGS) $(LDFLAGS) -o rtacct rtacct.c $(LIBNETLINK) -lm
+rtacct: rtacct.o
+ $(QUIET_CC)$(CC) $(LDFLAGS) -o rtacct rtacct.o $(LIBNETLINK) -lm
-arpd: arpd.c
- $(QUIET_CC)$(CC) $(CFLAGS) -I$(DBM_INCLUDE) $(LDFLAGS) -o arpd arpd.c $(LIBNETLINK) -ldb -lpthread
+arpd: arpd.o
+ $(QUIET_CC)$(CC) $(LDFLAGS) -o arpd arpd.o $(LIBNETLINK) -ldb -lpthread
ssfilter.c: ssfilter.y
$(QUIET_YACC)bison ssfilter.y -o ssfilter.c
--
2.12.3
^ permalink raw reply related
* Re: [PATCH v2 05/11] mtd: cfi: reduce stack size with KASAN
From: Boris Brezillon @ 2017-08-04 10:56 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Andrew Morton, kasan-dev, Dmitry Vyukov, Alexander Potapenko,
Andrey Ryabinin, Networking, Linux Kernel Mailing List,
Arend van Spriel, David Woodhouse, Brian Norris, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, linux-mtd
In-Reply-To: <CAK8P3a2p6w_yx82SC796n6dqZwE1=s7Mad5TLqcdK=pJ0Uiy=w@mail.gmail.com>
On Fri, 4 Aug 2017 11:09:53 +0200
Arnd Bergmann <arnd@arndb.de> wrote:
> On Fri, Aug 4, 2017 at 9:42 AM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Wed, 14 Jun 2017 23:15:40 +0200
> > Arnd Bergmann <arnd@arndb.de> wrote:
> >
> >> When CONFIG_KASAN is used, we consume a lot of extra stack space:
> >>
> >> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'do_write_buffer':
> >> drivers/mtd/chips/cfi_cmdset_0020.c:603:1: error: the frame size of 2184 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> >> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_erase_varsize':
> >> drivers/mtd/chips/cfi_cmdset_0020.c:972:1: error: the frame size of 1936 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> >> drivers/mtd/chips/cfi_cmdset_0001.c: In function 'do_write_buffer':
> >> drivers/mtd/chips/cfi_cmdset_0001.c:1841:1: error: the frame size of 1776 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> >>
> >> This marks some functions as noinline_if_stackbloat to keep reduce the
> >> overall stack size.
> >>
> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >> ---
> >> drivers/mtd/chips/cfi_cmdset_0020.c | 8 ++++----
> >> include/linux/mtd/map.h | 8 ++++----
> >> 2 files changed, 8 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c b/drivers/mtd/chips/cfi_cmdset_0020.c
> >> index 7d342965f392..5eee5e883f55 100644
> >> --- a/drivers/mtd/chips/cfi_cmdset_0020.c
> >> +++ b/drivers/mtd/chips/cfi_cmdset_0020.c
> >> @@ -244,7 +244,7 @@ static struct mtd_info *cfi_staa_setup(struct map_info *map)
> >> }
> >>
> >>
> >> -static inline int do_read_onechip(struct map_info *map, struct flchip *chip, loff_t adr, size_t len, u_char *buf)
> >> +static noinline_if_stackbloat int do_read_onechip(struct map_info *map, struct flchip *chip, loff_t adr, size_t len, u_char *buf)
> >
> > Why do we even need to mark those functions inline in the first place?
> > Isn't the compiler smart enough to decide when it should inline things?
>
> I'm pretty sure it doesn't need the 'inline' keywork. I see that the code was
> addedlike this in linux-2.4.0-test3pre3 along with the rest of the mtd layer,
> so it has always been 'inline' and nobody ever bothered to remove that
> during later cleanups.
>
> We could probably just mark this function as 'noinline' here and never
> inline it,
> but I would rather not add more than one variant of noinline_if_stackbloat:
> almost all other users of noinline_if_stackbloat are for functions that have
> to be inline in normal builds, so it is defined as being either
> 'inline' or 'noinline'
> depending on whether KASAN is active.
Okay. Let's keep it like that then.
^ 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