* Re: [PATCH net] tcp: refresh skb timestamp at retransmit time
From: David Miller @ 2016-05-10 19:59 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, ycheng
In-Reply-To: <1462852516.23934.46.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 09 May 2016 20:55:16 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> In the very unlikely case __tcp_retransmit_skb() can not use the cloning
> done in tcp_transmit_skb(), we need to refresh skb_mstamp before doing
> the copy and transmit, otherwise TCP TS val will be an exact copy of
> original transmit.
>
> Fixes: 7faee5c0d514 ("tcp: remove TCP_SKB_CB(skb)->when")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] net: phylib: fix interrupts re-enablement in phy_start
From: David Miller @ 2016-05-10 20:01 UTC (permalink / raw)
To: shh.xie; +Cc: netdev, f.fainelli, Shaohui.Xie
In-Reply-To: <1462873346-46457-1-git-send-email-shh.xie@gmail.com>
From: <shh.xie@gmail.com>
Date: Tue, 10 May 2016 17:42:26 +0800
> From: Shaohui Xie <Shaohui.Xie@nxp.com>
>
> If phy was suspended and is starting, current driver always enable
> phy's interrupts, if phy works in polling, phy can raise unexpected
> interrupt which will not be handled, the interrupt will block system
> enter suspend again. So interrupts should only be re-enabled if phy
> works in interrupt.
>
> Signed-off-by: Shaohui Xie <Shaohui.Xie@nxp.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next v2] ila: ipv6/ila: fix nlsize calculation for lwtunnel
From: David Miller @ 2016-05-10 20:01 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: tom, netdev
In-Reply-To: <1462874192-8360-1-git-send-email-nicolas.dichtel@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Tue, 10 May 2016 11:56:32 +0200
> From: Tom Herbert <tom@herbertland.com>
>
> The handler 'ila_fill_encap_info' adds two attributes: ILA_ATTR_LOCATOR
> and ILA_ATTR_CSUM_MODE.
>
> nla_total_size_64bit() must be use for ILA_ATTR_LOCATOR.
>
> Also, do nla_put_u8 instead of nla_put_u64 for ILA_ATTR_CSUM_MODE.
>
> Fixes: f13a82d87b21 ("ipv6: use nla_put_u64_64bit()")
> Fixes: 90bfe662db13 ("ila: add checksum neutral ILA translations")
> Reported-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Signed-off-by: Tom Herbert <tom@herbertland.com>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] skbuff: remove unused variable `doff'
From: David Miller @ 2016-05-10 20:05 UTC (permalink / raw)
To: sowmini.varadhan; +Cc: netdev, daniel
In-Reply-To: <20160510163808.GN13183@oracle.com>
From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Date: Tue, 10 May 2016 12:38:08 -0400
> There are two instances of an unused variable, `doff' added by
> commit 6fa01ccd8830 ("skbuff: Add pskb_extract() helper function")
> in pskb_carve_inside_header() and pskb_carve_inside_nonlinear().
> Remove these instances, they are not used.
>
> Reported by: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Applied.
^ permalink raw reply
* [PATCH v3 net-next] tcp: replace cnt & rtt with struct in pkts_acked()
From: Lawrence Brakmo @ 2016-05-10 20:11 UTC (permalink / raw)
To: netdev; +Cc: Kernel Team, Neal Cardwell, Eric Dumazet, Yuchung Cheng
Replace 2 arguments (cnt and rtt) in the congestion control modules'
pkts_acked() function with a struct. This will allow adding more
information without having to modify existing congestion control
modules (tcp_nv in particular needs bytes in flight when packet
was sent).
As proposed by Neal Cardwell in his comments to the tcp_nv patch.
Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
---
include/net/tcp.h | 7 ++++++-
net/ipv4/tcp_bic.c | 6 +++---
net/ipv4/tcp_cdg.c | 14 +++++++-------
net/ipv4/tcp_cubic.c | 6 +++---
net/ipv4/tcp_htcp.c | 10 +++++-----
net/ipv4/tcp_illinois.c | 20 ++++++++++----------
net/ipv4/tcp_input.c | 8 ++++++--
net/ipv4/tcp_lp.c | 6 +++---
net/ipv4/tcp_vegas.c | 6 +++---
net/ipv4/tcp_vegas.h | 2 +-
net/ipv4/tcp_veno.c | 7 ++++---
net/ipv4/tcp_westwood.c | 7 ++++---
net/ipv4/tcp_yeah.c | 7 ++++---
13 files changed, 59 insertions(+), 47 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 24ec804..dc588c3 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -849,6 +849,11 @@ enum tcp_ca_ack_event_flags {
union tcp_cc_info;
+struct ack_sample {
+ u32 pkts_acked;
+ s32 rtt_us;
+};
+
struct tcp_congestion_ops {
struct list_head list;
u32 key;
@@ -872,7 +877,7 @@ struct tcp_congestion_ops {
/* new value of cwnd after loss (optional) */
u32 (*undo_cwnd)(struct sock *sk);
/* hook for packet ack accounting (optional) */
- void (*pkts_acked)(struct sock *sk, u32 num_acked, s32 rtt_us);
+ void (*pkts_acked)(struct sock *sk, const struct ack_sample *sample);
/* get info for inet_diag (optional) */
size_t (*get_info)(struct sock *sk, u32 ext, int *attr,
union tcp_cc_info *info);
diff --git a/net/ipv4/tcp_bic.c b/net/ipv4/tcp_bic.c
index fd1405d..36087bc 100644
--- a/net/ipv4/tcp_bic.c
+++ b/net/ipv4/tcp_bic.c
@@ -197,15 +197,15 @@ static void bictcp_state(struct sock *sk, u8 new_state)
/* Track delayed acknowledgment ratio using sliding window
* ratio = (15*ratio + sample) / 16
*/
-static void bictcp_acked(struct sock *sk, u32 cnt, s32 rtt)
+static void bictcp_acked(struct sock *sk, const struct ack_sample *sample)
{
const struct inet_connection_sock *icsk = inet_csk(sk);
if (icsk->icsk_ca_state == TCP_CA_Open) {
struct bictcp *ca = inet_csk_ca(sk);
- cnt -= ca->delayed_ack >> ACK_RATIO_SHIFT;
- ca->delayed_ack += cnt;
+ ca->delayed_ack += sample->pkts_acked -
+ (ca->delayed_ack >> ACK_RATIO_SHIFT);
}
}
diff --git a/net/ipv4/tcp_cdg.c b/net/ipv4/tcp_cdg.c
index ccce8a5..03725b2 100644
--- a/net/ipv4/tcp_cdg.c
+++ b/net/ipv4/tcp_cdg.c
@@ -294,12 +294,12 @@ static void tcp_cdg_cong_avoid(struct sock *sk, u32 ack, u32 acked)
ca->shadow_wnd = max(ca->shadow_wnd, ca->shadow_wnd + incr);
}
-static void tcp_cdg_acked(struct sock *sk, u32 num_acked, s32 rtt_us)
+static void tcp_cdg_acked(struct sock *sk, const struct ack_sample *sample)
{
struct cdg *ca = inet_csk_ca(sk);
struct tcp_sock *tp = tcp_sk(sk);
- if (rtt_us <= 0)
+ if (sample->rtt_us <= 0)
return;
/* A heuristic for filtering delayed ACKs, adapted from:
@@ -307,20 +307,20 @@ static void tcp_cdg_acked(struct sock *sk, u32 num_acked, s32 rtt_us)
* delay and rate based TCP mechanisms." TR 100219A. CAIA, 2010.
*/
if (tp->sacked_out == 0) {
- if (num_acked == 1 && ca->delack) {
+ if (sample->pkts_acked == 1 && ca->delack) {
/* A delayed ACK is only used for the minimum if it is
* provenly lower than an existing non-zero minimum.
*/
- ca->rtt.min = min(ca->rtt.min, rtt_us);
+ ca->rtt.min = min(ca->rtt.min, sample->rtt_us);
ca->delack--;
return;
- } else if (num_acked > 1 && ca->delack < 5) {
+ } else if (sample->pkts_acked > 1 && ca->delack < 5) {
ca->delack++;
}
}
- ca->rtt.min = min_not_zero(ca->rtt.min, rtt_us);
- ca->rtt.max = max(ca->rtt.max, rtt_us);
+ ca->rtt.min = min_not_zero(ca->rtt.min, sample->rtt_us);
+ ca->rtt.max = max(ca->rtt.max, sample->rtt_us);
}
static u32 tcp_cdg_ssthresh(struct sock *sk)
diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
index 0ce946e..c99230e 100644
--- a/net/ipv4/tcp_cubic.c
+++ b/net/ipv4/tcp_cubic.c
@@ -437,21 +437,21 @@ static void hystart_update(struct sock *sk, u32 delay)
/* Track delayed acknowledgment ratio using sliding window
* ratio = (15*ratio + sample) / 16
*/
-static void bictcp_acked(struct sock *sk, u32 cnt, s32 rtt_us)
+static void bictcp_acked(struct sock *sk, const struct ack_sample *sample)
{
const struct tcp_sock *tp = tcp_sk(sk);
struct bictcp *ca = inet_csk_ca(sk);
u32 delay;
/* Some calls are for duplicates without timetamps */
- if (rtt_us < 0)
+ if (sample->rtt_us < 0)
return;
/* Discard delay samples right after fast recovery */
if (ca->epoch_start && (s32)(tcp_time_stamp - ca->epoch_start) < HZ)
return;
- delay = (rtt_us << 3) / USEC_PER_MSEC;
+ delay = (sample->rtt_us << 3) / USEC_PER_MSEC;
if (delay == 0)
delay = 1;
diff --git a/net/ipv4/tcp_htcp.c b/net/ipv4/tcp_htcp.c
index 82f0d9e..4a4d8e7 100644
--- a/net/ipv4/tcp_htcp.c
+++ b/net/ipv4/tcp_htcp.c
@@ -99,7 +99,7 @@ static inline void measure_rtt(struct sock *sk, u32 srtt)
}
static void measure_achieved_throughput(struct sock *sk,
- u32 pkts_acked, s32 rtt)
+ const struct ack_sample *sample)
{
const struct inet_connection_sock *icsk = inet_csk(sk);
const struct tcp_sock *tp = tcp_sk(sk);
@@ -107,10 +107,10 @@ static void measure_achieved_throughput(struct sock *sk,
u32 now = tcp_time_stamp;
if (icsk->icsk_ca_state == TCP_CA_Open)
- ca->pkts_acked = pkts_acked;
+ ca->pkts_acked = sample->pkts_acked;
- if (rtt > 0)
- measure_rtt(sk, usecs_to_jiffies(rtt));
+ if (sample->rtt_us > 0)
+ measure_rtt(sk, usecs_to_jiffies(sample->rtt_us));
if (!use_bandwidth_switch)
return;
@@ -122,7 +122,7 @@ static void measure_achieved_throughput(struct sock *sk,
return;
}
- ca->packetcount += pkts_acked;
+ ca->packetcount += sample->pkts_acked;
if (ca->packetcount >= tp->snd_cwnd - (ca->alpha >> 7 ? : 1) &&
now - ca->lasttime >= ca->minRTT &&
diff --git a/net/ipv4/tcp_illinois.c b/net/ipv4/tcp_illinois.c
index 2ab9bbb..77ad119 100644
--- a/net/ipv4/tcp_illinois.c
+++ b/net/ipv4/tcp_illinois.c
@@ -82,30 +82,30 @@ static void tcp_illinois_init(struct sock *sk)
}
/* Measure RTT for each ack. */
-static void tcp_illinois_acked(struct sock *sk, u32 pkts_acked, s32 rtt)
+static void tcp_illinois_acked(struct sock *sk, const struct ack_sample *sample)
{
struct illinois *ca = inet_csk_ca(sk);
- ca->acked = pkts_acked;
+ ca->acked = sample->pkts_acked;
/* dup ack, no rtt sample */
- if (rtt < 0)
+ if (sample->rtt_us < 0)
return;
/* ignore bogus values, this prevents wraparound in alpha math */
- if (rtt > RTT_MAX)
- rtt = RTT_MAX;
+ if (sample->rtt_us > RTT_MAX)
+ sample->rtt_us = RTT_MAX;
/* keep track of minimum RTT seen so far */
- if (ca->base_rtt > rtt)
- ca->base_rtt = rtt;
+ if (ca->base_rtt > sample->rtt_us)
+ ca->base_rtt = sample->rtt_us;
/* and max */
- if (ca->max_rtt < rtt)
- ca->max_rtt = rtt;
+ if (ca->max_rtt < sample->rtt_us)
+ ca->max_rtt = sample->rtt_us;
++ca->cnt_rtt;
- ca->sum_rtt += rtt;
+ ca->sum_rtt += sample->rtt_us;
}
/* Maximum queuing delay */
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index a914e06..d6c8f4cd0 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3248,8 +3248,12 @@ static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets,
tcp_rearm_rto(sk);
}
- if (icsk->icsk_ca_ops->pkts_acked)
- icsk->icsk_ca_ops->pkts_acked(sk, pkts_acked, ca_rtt_us);
+ if (icsk->icsk_ca_ops->pkts_acked) {
+ struct ack_sample sample = { .pkts_acked = pkts_acked,
+ .rtt_us = ca_rtt_us };
+
+ icsk->icsk_ca_ops->pkts_acked(sk, &sample);
+ }
#if FASTRETRANS_DEBUG > 0
WARN_ON((int)tp->sacked_out < 0);
diff --git a/net/ipv4/tcp_lp.c b/net/ipv4/tcp_lp.c
index 1e70fa8..c67ece1 100644
--- a/net/ipv4/tcp_lp.c
+++ b/net/ipv4/tcp_lp.c
@@ -260,13 +260,13 @@ static void tcp_lp_rtt_sample(struct sock *sk, u32 rtt)
* newReno in increase case.
* We work it out by following the idea from TCP-LP's paper directly
*/
-static void tcp_lp_pkts_acked(struct sock *sk, u32 num_acked, s32 rtt_us)
+static void tcp_lp_pkts_acked(struct sock *sk, const struct ack_sample *sample)
{
struct tcp_sock *tp = tcp_sk(sk);
struct lp *lp = inet_csk_ca(sk);
- if (rtt_us > 0)
- tcp_lp_rtt_sample(sk, rtt_us);
+ if (sample->rtt_us > 0)
+ tcp_lp_rtt_sample(sk, sample->rtt_us);
/* calc inference */
if (tcp_time_stamp > tp->rx_opt.rcv_tsecr)
diff --git a/net/ipv4/tcp_vegas.c b/net/ipv4/tcp_vegas.c
index 13951c4..4c4bac1 100644
--- a/net/ipv4/tcp_vegas.c
+++ b/net/ipv4/tcp_vegas.c
@@ -107,16 +107,16 @@ EXPORT_SYMBOL_GPL(tcp_vegas_init);
* o min-filter RTT samples from a much longer window (forever for now)
* to find the propagation delay (baseRTT)
*/
-void tcp_vegas_pkts_acked(struct sock *sk, u32 cnt, s32 rtt_us)
+void tcp_vegas_pkts_acked(struct sock *sk, const struct ack_sample *sample)
{
struct vegas *vegas = inet_csk_ca(sk);
u32 vrtt;
- if (rtt_us < 0)
+ if (sample->rtt_us < 0)
return;
/* Never allow zero rtt or baseRTT */
- vrtt = rtt_us + 1;
+ vrtt = sample->rtt_us + 1;
/* Filter to find propagation delay: */
if (vrtt < vegas->baseRTT)
diff --git a/net/ipv4/tcp_vegas.h b/net/ipv4/tcp_vegas.h
index ef9da53..248cfc0 100644
--- a/net/ipv4/tcp_vegas.h
+++ b/net/ipv4/tcp_vegas.h
@@ -17,7 +17,7 @@ struct vegas {
void tcp_vegas_init(struct sock *sk);
void tcp_vegas_state(struct sock *sk, u8 ca_state);
-void tcp_vegas_pkts_acked(struct sock *sk, u32 cnt, s32 rtt_us);
+void tcp_vegas_pkts_acked(struct sock *sk, const struct ack_sample *sample);
void tcp_vegas_cwnd_event(struct sock *sk, enum tcp_ca_event event);
size_t tcp_vegas_get_info(struct sock *sk, u32 ext, int *attr,
union tcp_cc_info *info);
diff --git a/net/ipv4/tcp_veno.c b/net/ipv4/tcp_veno.c
index 0d094b9..40171e1 100644
--- a/net/ipv4/tcp_veno.c
+++ b/net/ipv4/tcp_veno.c
@@ -69,16 +69,17 @@ static void tcp_veno_init(struct sock *sk)
}
/* Do rtt sampling needed for Veno. */
-static void tcp_veno_pkts_acked(struct sock *sk, u32 cnt, s32 rtt_us)
+static void tcp_veno_pkts_acked(struct sock *sk,
+ const struct ack_sample *sample)
{
struct veno *veno = inet_csk_ca(sk);
u32 vrtt;
- if (rtt_us < 0)
+ if (sample->rtt_us < 0)
return;
/* Never allow zero rtt or baseRTT */
- vrtt = rtt_us + 1;
+ vrtt = sample->rtt_us + 1;
/* Filter to find propagation delay: */
if (vrtt < veno->basertt)
diff --git a/net/ipv4/tcp_westwood.c b/net/ipv4/tcp_westwood.c
index c10732e..4b03a2e 100644
--- a/net/ipv4/tcp_westwood.c
+++ b/net/ipv4/tcp_westwood.c
@@ -99,12 +99,13 @@ static void westwood_filter(struct westwood *w, u32 delta)
* Called after processing group of packets.
* but all westwood needs is the last sample of srtt.
*/
-static void tcp_westwood_pkts_acked(struct sock *sk, u32 cnt, s32 rtt)
+static void tcp_westwood_pkts_acked(struct sock *sk,
+ const struct ack_sample *sample)
{
struct westwood *w = inet_csk_ca(sk);
- if (rtt > 0)
- w->rtt = usecs_to_jiffies(rtt);
+ if (sample->rtt_us > 0)
+ w->rtt = usecs_to_jiffies(sample->rtt_us);
}
/*
diff --git a/net/ipv4/tcp_yeah.c b/net/ipv4/tcp_yeah.c
index 3e6a472..028eb04 100644
--- a/net/ipv4/tcp_yeah.c
+++ b/net/ipv4/tcp_yeah.c
@@ -56,15 +56,16 @@ static void tcp_yeah_init(struct sock *sk)
tp->snd_cwnd_clamp = min_t(u32, tp->snd_cwnd_clamp, 0xffffffff/128);
}
-static void tcp_yeah_pkts_acked(struct sock *sk, u32 pkts_acked, s32 rtt_us)
+static void tcp_yeah_pkts_acked(struct sock *sk,
+ const struct ack_sample *sample)
{
const struct inet_connection_sock *icsk = inet_csk(sk);
struct yeah *yeah = inet_csk_ca(sk);
if (icsk->icsk_ca_state == TCP_CA_Open)
- yeah->pkts_acked = pkts_acked;
+ yeah->pkts_acked = sample->pkts_acked;
- tcp_vegas_pkts_acked(sk, pkts_acked, rtt_us);
+ tcp_vegas_pkts_acked(sk, sample);
}
static void tcp_yeah_cong_avoid(struct sock *sk, u32 ack, u32 acked)
--
2.8.0.rc2
^ permalink raw reply related
* Re: [RFC PATCH 0/2] net: threadable napi poll loop
From: Paolo Abeni @ 2016-05-10 20:22 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, David S. Miller, Eric Dumazet, Jiri Pirko,
Daniel Borkmann, Alexei Starovoitov, Alexander Duyck, Tom Herbert,
Peter Zijlstra, Ingo Molnar, Rik van Riel, Hannes Frederic Sowa,
linux-kernel
In-Reply-To: <1462896539.23934.93.camel@edumazet-glaptop3.roam.corp.google.com>
On Tue, 2016-05-10 at 09:08 -0700, Eric Dumazet wrote:
> On Tue, 2016-05-10 at 18:03 +0200, Paolo Abeni wrote:
>
> > If a single core host is under network flood, i.e. ksoftirqd is
> > scheduled and it eventually (after processing ~640 packets) will let the
> > user space process run. The latter will execute a syscall to receive a
> > packet, which will have to disable/enable bh at least once and that will
> > cause the processing of another ~640 packets. To receive a single packet
> > in user space, the kernel has to process more than one thousand packets.
>
> Looks you found the bug then. Have you tried to fix it ?
The core functionality is implemented in ~100 lines of code, is that
the kind of bloat that do concerns you ?
That could probably be improved removing some code duplication, i.e.
factorizing napi_thread_wait() with irq_wait_for_interrupt() and
possibly napi_threaded_poll() with net_rx_action().
If the additional test inside napi_schedule() is really scaring, it can
be guarded with a static_key.
The ksoftirq and the local_bh_enable() design are the root of the
problem, they need to be touched/affected to solve it.
We actually experimented several different options.
Limiting the amount of work performed by local_bh_enable() somewhat
mitigate the issue, but it adds just another kernel parameter difficult
to be tuned.
Running the softirq loop exclusively inside the ksoftirqd will solve the
issue, but this is a very invasive approach, affecting all others
subsystem.
The above can be restricted to the net_rx_action only (i.e. running
net_rx_action always in ksoftirqd context). The related patch isn't
really much simpler than this and will add at least the same number of
additional tests in fast path.
Running the napi loop in a thread that can be migrated gives additional
benefit in the hyper-visor/VM scenario, which can't be achieved
elsewhere.
Would you consider the threaded irq alternative more viable ?
Cheers,
Paolo
^ permalink raw reply
* Re: [RFC PATCH 0/2] net: threadable napi poll loop
From: Paolo Abeni @ 2016-05-10 20:41 UTC (permalink / raw)
To: Thomas Gleixner
Cc: netdev, David S. Miller, Eric Dumazet, Jiri Pirko,
Daniel Borkmann, Alexei Starovoitov, Alexander Duyck, Tom Herbert,
Peter Zijlstra, Ingo Molnar, Rik van Riel, Hannes Frederic Sowa,
linux-kernel
In-Reply-To: <alpine.DEB.2.11.1605101655100.3540@nanos>
On Tue, 2016-05-10 at 17:57 +0200, Thomas Gleixner wrote:
> On Tue, 10 May 2016, Paolo Abeni wrote:
>
> Nice patch set and very promising results!
>
> > At this point we are not really sure if we should go with this simpler
> > approach by putting NAPI itself into kthreads or leverage the threadirqs
> > function by putting the whole interrupt into a thread and signaling NAPI
> > that it does not reschedule itself in a softirq but to simply run at
> > this particular context of the interrupt handler.
> >
> > While the threaded irq way seems to better integrate into the kernel and
> > also other devices could move their interrupts into the threads easily
> > on a common policy, we don't know how to really express the necessary
> > knobs with the current device driver model (module parameters, sysfs
> > attributes, etc.). This is where we would like to hear some opinions.
> > NAPI would e.g. have to query the kernel if the particular IRQ/MSI if it
> > should be scheduled in a softirq or in a thread, so we don't have to
> > rewrite all device drivers. This might even be needed on a per rx-queue
> > granularity.
>
> Utilizing threaded irqs should be halfways simple even without touching the
> device driver at all.
>
> We can do the switch to threading in two ways:
>
> 1) Let the driver request the interrupt(s) as it does now and then have a
> /proc/irq/NNN/threaded file which converts it to a threaded interrupt on
> the fly. That should be fairly trivial.
>
> 2) Let the driver request the interrupt(s) as it does now and retrieve the
> interrupt number which belongs to the device/queue from the network core
> and let the irq core switch it over to threaded.
Thank you for the feedback.
We actually experimented something similar to (2). In our implementation
we needed a per device chunk of code to do the actual irq number ->
queue mapping (and than we performed as well the switch in the device
code).
> You surely need some way to figure out whether the interrupt is threaded when
> you set up the device in order to flag your napi struct, but that should be
> not too hard to achieve.
This is the part that required per device changes and complicated a bit
the implementation. We can research further to simplify it, according to
the overall discussion.
Cheers,
Paolo
^ permalink raw reply
* Re: [RFC PATCH 0/2] net: threadable napi poll loop
From: David Miller @ 2016-05-10 20:45 UTC (permalink / raw)
To: pabeni
Cc: eric.dumazet, netdev, edumazet, jiri, daniel, ast, aduyck, tom,
peterz, mingo, riel, hannes, linux-kernel
In-Reply-To: <1462911770.5333.11.camel@redhat.com>
From: Paolo Abeni <pabeni@redhat.com>
Date: Tue, 10 May 2016 22:22:50 +0200
> On Tue, 2016-05-10 at 09:08 -0700, Eric Dumazet wrote:
>> On Tue, 2016-05-10 at 18:03 +0200, Paolo Abeni wrote:
>>
>> > If a single core host is under network flood, i.e. ksoftirqd is
>> > scheduled and it eventually (after processing ~640 packets) will let the
>> > user space process run. The latter will execute a syscall to receive a
>> > packet, which will have to disable/enable bh at least once and that will
>> > cause the processing of another ~640 packets. To receive a single packet
>> > in user space, the kernel has to process more than one thousand packets.
>>
>> Looks you found the bug then. Have you tried to fix it ?
...
> The ksoftirq and the local_bh_enable() design are the root of the
> problem, they need to be touched/affected to solve it.
That's not what I read from your description, processing 640 packets
before going to ksoftirqd seems to the be the absolute root problem.
^ permalink raw reply
* Re: [RFC PATCH 0/2] net: threadable napi poll loop
From: Hannes Frederic Sowa @ 2016-05-10 20:46 UTC (permalink / raw)
To: Eric Dumazet, Paolo Abeni
Cc: netdev, David S. Miller, Eric Dumazet, Jiri Pirko,
Daniel Borkmann, Alexei Starovoitov, Alexander Duyck, Tom Herbert,
Peter Zijlstra, Ingo Molnar, Rik van Riel, linux-kernel
In-Reply-To: <1462890590.23934.68.camel@edumazet-glaptop3.roam.corp.google.com>
Hello,
On 10.05.2016 16:29, Eric Dumazet wrote:
> On Tue, 2016-05-10 at 16:11 +0200, Paolo Abeni wrote:
>> Currently, the softirq loop can be scheduled both inside the ksofirqd kernel
>> thread and inside any running process. This makes nearly impossible for the
>> process scheduler to balance in a fair way the amount of time that
>> a given core spends performing the softirq loop.
>>
>> Under high network load, the softirq loop can take nearly 100% of a given CPU,
>> leaving very little time for use space processing. On single core hosts, this
>> means that the user space can nearly starve; for example super_netperf
>> UDP_STREAM tests towards a remote single core vCPU guest[1] can measure an
>> aggregated throughput of a few thousands pps, and the same behavior can be
>> reproduced even on bare-metal, eventually simulating a single core with taskset
>> and/or sysfs configuration.
>
> I hate these patches and ideas guys, sorry. That is before my breakfast,
> but still...
:)
> I have enough hard time dealing with loads where ksoftirqd has to
> compete with user threads that thought that playing with priorities was
> a nice idea.
We tried a lot of approaches so far and this seemed to be the best
architectural RFC we could post. I was quite surprised to see such good
performance numbers with threaded NAPI, thus I think it could be a way
forward.
Your mentioned problem above seems to be a configuration mistake, no?
Otherwise isn't that something user space/cgroups might solve?
> Guess what, when they lose networking they complain.
>
> We already have ksoftirqd to normally cope with the case you are
> describing.
Indeed, but the time until we wake up ksoftirqd can be already quite
long and for every packet we get in udp_recvmsg the local_bh_enable call
let's us pick up quite a lot of new packets, which we drop before user
space can make any progress. By being more fair between user space and
"napid" we hoped to solve this. We also want more feedback from the
scheduler people, so we Cc'ed them also.
> If it is not working as intended, please identify the bugs and fix them,
> instead of adding yet another tests in fast path and extra complexity in
> the stack.
We could use _local_bh_enable instead of local_bh_enable in udp_recvmsg,
which certainly wouldn't branch down to softirqs as often, but this
feels wrong to me and certainly is.
After the discussion on netdev@ with Peter Hurley here [1] about
"Softirq priority inversion from "softirq: reduce latencies"", we didn't
want to propose some patch looking like this again, but this could help.
The idea would be to limit the number we recheck for softirqs but give
back control to user space.
[1] https://lkml.org/lkml/2016/2/27/152
If I remember local_bh_enable in kernel-rt processes one softirq
directly and defers its work to ksoftirqd much more quickly.
> In the one vcpu case, allowing the user thread to consume more UDP
> packets from the target UDP socket will also make your NIC drop more
> packets, that are not necessarily packets for the same socket.
>
> So you are shifting the attack to a different target,
> at the expense of more kernel bloat.
I agree here, but I don't think this patch particularly is a lot of
bloat and something very interesting people can play with and extend upon.
Thanks,
Hannes
^ permalink raw reply
* [net PATCH v2 0/6] net sched: Fix broken late binding of actions
From: Jamal Hadi Salim @ 2016-05-10 20:49 UTC (permalink / raw)
To: davem; +Cc: netdev, xiyou.wangcong, jiri, alexander.duyck, Jamal Hadi Salim
From: Jamal Hadi Salim <jhs@mojatatu.com>
Some actions were broken in allowing for late binding of actions.
Late binding workflow is as follows:
a) create an action and provide all necessary parameters for it
Optionally provide an index or let the kernel give you one.
Example:
sudo tc actions add action police rate 1kbit burst 90k drop index 1
b) later on bind to the pre-created action from a filter definition
by merely specifying the index.
Example:
sudo tc filter add dev lo parent ffff: protocol ip prio 8 \
u32 match ip src 127.0.0.8/32 flowid 1:8 action police index 1
Jamal Hadi Salim (6):
net sched: vlan action fix late binding
net sched: ipt action fix late binding
net sched: mirred action fix late binding
net sched: simple action fix late binding
net sched: skbedit action fix late binding
net sched: ife action fix late binding
net/sched/act_ife.c | 14 ++++++++++----
net/sched/act_ipt.c | 19 ++++++++++++-------
net/sched/act_mirred.c | 19 +++++++++++++------
net/sched/act_simple.c | 18 ++++++++++++------
net/sched/act_skbedit.c | 18 +++++++++++-------
net/sched/act_vlan.c | 22 ++++++++++++++++------
6 files changed, 74 insertions(+), 36 deletions(-)
--
1.9.1
^ permalink raw reply
* [net PATCH v2 1/6] net sched: vlan action fix late binding
From: Jamal Hadi Salim @ 2016-05-10 20:49 UTC (permalink / raw)
To: davem; +Cc: netdev, xiyou.wangcong, jiri, alexander.duyck, Jamal Hadi Salim
In-Reply-To: <1462913371-9699-1-git-send-email-jhs@emojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
Late vlan action binding was broken and is fixed with this patch.
//add a vlan action to pop and give it an instance id of 1
sudo tc actions add action vlan pop index 1
//create filter which binds to vlan action id 1
sudo tc filter add dev $DEV parent ffff: protocol ip prio 1 u32 \
match ip dst 17.0.0.1/32 flowid 1:1 action vlan index 1
current message(before bug fix) was:
RTNETLINK answers: Invalid argument
We have an error talking to the kernel
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/act_vlan.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index bab8ae0..c45f926 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -77,7 +77,7 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
int action;
__be16 push_vid = 0;
__be16 push_proto = 0;
- int ret = 0;
+ int ret = 0, exists = 0;
int err;
if (!nla)
@@ -90,15 +90,25 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
if (!tb[TCA_VLAN_PARMS])
return -EINVAL;
parm = nla_data(tb[TCA_VLAN_PARMS]);
+ exists = tcf_hash_check(tn, parm->index, a, bind);
+ if (exists && bind)
+ return 0;
+
switch (parm->v_action) {
case TCA_VLAN_ACT_POP:
break;
case TCA_VLAN_ACT_PUSH:
- if (!tb[TCA_VLAN_PUSH_VLAN_ID])
+ if (!tb[TCA_VLAN_PUSH_VLAN_ID]) {
+ if (exists)
+ tcf_hash_release(a, bind);
return -EINVAL;
+ }
push_vid = nla_get_u16(tb[TCA_VLAN_PUSH_VLAN_ID]);
- if (push_vid >= VLAN_VID_MASK)
+ if (push_vid >= VLAN_VID_MASK) {
+ if (exists)
+ tcf_hash_release(a, bind);
return -ERANGE;
+ }
if (tb[TCA_VLAN_PUSH_VLAN_PROTOCOL]) {
push_proto = nla_get_be16(tb[TCA_VLAN_PUSH_VLAN_PROTOCOL]);
@@ -114,11 +124,13 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
}
break;
default:
+ if (exists)
+ tcf_hash_release(a, bind);
return -EINVAL;
}
action = parm->v_action;
- if (!tcf_hash_check(tn, parm->index, a, bind)) {
+ if (!exists) {
ret = tcf_hash_create(tn, parm->index, est, a,
sizeof(*v), bind, false);
if (ret)
@@ -126,8 +138,6 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
ret = ACT_P_CREATED;
} else {
- if (bind)
- return 0;
tcf_hash_release(a, bind);
if (!ovr)
return -EEXIST;
--
1.9.1
^ permalink raw reply related
* [net PATCH v2 2/6] net sched: ipt action fix late binding
From: Jamal Hadi Salim @ 2016-05-10 20:49 UTC (permalink / raw)
To: davem; +Cc: netdev, xiyou.wangcong, jiri, alexander.duyck, Jamal Hadi Salim
In-Reply-To: <1462913371-9699-1-git-send-email-jhs@emojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
This was broken and is fixed with this patch.
//add an ipt action and give it an instance id of 1
sudo tc actions add action ipt -j mark --set-mark 2 index 1
//create a filter which binds to ipt action id 1
sudo tc filter add dev $DEV parent ffff: protocol ip prio 1 u32\
match ip dst 17.0.0.1/32 flowid 1:10 action ipt index 1
Message before bug fix was:
RTNETLINK answers: Invalid argument
We have an error talking to the kernel
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/act_ipt.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index 350e134..8b52700 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -96,7 +96,7 @@ static int __tcf_ipt_init(struct tc_action_net *tn, struct nlattr *nla,
struct tcf_ipt *ipt;
struct xt_entry_target *td, *t;
char *tname;
- int ret = 0, err;
+ int ret = 0, err, exists = 0;
u32 hook = 0;
u32 index = 0;
@@ -107,18 +107,23 @@ static int __tcf_ipt_init(struct tc_action_net *tn, struct nlattr *nla,
if (err < 0)
return err;
- if (tb[TCA_IPT_HOOK] == NULL)
- return -EINVAL;
- if (tb[TCA_IPT_TARG] == NULL)
+ if (tb[TCA_IPT_INDEX] != NULL)
+ index = nla_get_u32(tb[TCA_IPT_INDEX]);
+
+ exists = tcf_hash_check(tn, index, a, bind);
+ if (exists && bind)
+ return 0;
+
+ if (tb[TCA_IPT_HOOK] == NULL || tb[TCA_IPT_TARG] == NULL) {
+ if (exists)
+ tcf_hash_release(a, bind);
return -EINVAL;
+ }
td = (struct xt_entry_target *)nla_data(tb[TCA_IPT_TARG]);
if (nla_len(tb[TCA_IPT_TARG]) < td->u.target_size)
return -EINVAL;
- if (tb[TCA_IPT_INDEX] != NULL)
- index = nla_get_u32(tb[TCA_IPT_INDEX]);
-
if (!tcf_hash_check(tn, index, a, bind)) {
ret = tcf_hash_create(tn, index, est, a, sizeof(*ipt), bind,
false);
--
1.9.1
^ permalink raw reply related
* [net PATCH v2 3/6] net sched: mirred action fix late binding
From: Jamal Hadi Salim @ 2016-05-10 20:49 UTC (permalink / raw)
To: davem; +Cc: netdev, xiyou.wangcong, jiri, alexander.duyck, Jamal Hadi Salim
In-Reply-To: <1462913371-9699-1-git-send-email-jhs@emojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
The process below was broken and is fixed with this patch.
//add an mirred action and give it an instance id of 1
sudo tc actions add action mirred egress mirror dev $MDEV index 1
//create a filter which binds to mirred action id 1
sudo tc filter add dev $DEV parent ffff: protocol ip prio 1 u32\
match ip dst 17.0.0.1/32 flowid 1:10 action mirred index 1
Message before bug fix was:
RTNETLINK answers: Invalid argument
We have an error talking to the kernel
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/act_mirred.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index e8a760c..8f3948d 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -61,7 +61,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
struct tc_mirred *parm;
struct tcf_mirred *m;
struct net_device *dev;
- int ret, ok_push = 0;
+ int ret, ok_push = 0, exists = 0;
if (nla == NULL)
return -EINVAL;
@@ -71,17 +71,27 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
if (tb[TCA_MIRRED_PARMS] == NULL)
return -EINVAL;
parm = nla_data(tb[TCA_MIRRED_PARMS]);
+
+ exists = tcf_hash_check(tn, parm->index, a, bind);
+ if (exists && bind)
+ return 0;
+
switch (parm->eaction) {
case TCA_EGRESS_MIRROR:
case TCA_EGRESS_REDIR:
break;
default:
+ if (exists)
+ tcf_hash_release(a, bind);
return -EINVAL;
}
if (parm->ifindex) {
dev = __dev_get_by_index(net, parm->ifindex);
- if (dev == NULL)
+ if (dev == NULL) {
+ if (exists)
+ tcf_hash_release(a, bind);
return -ENODEV;
+ }
switch (dev->type) {
case ARPHRD_TUNNEL:
case ARPHRD_TUNNEL6:
@@ -99,7 +109,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
dev = NULL;
}
- if (!tcf_hash_check(tn, parm->index, a, bind)) {
+ if (!exists) {
if (dev == NULL)
return -EINVAL;
ret = tcf_hash_create(tn, parm->index, est, a,
@@ -108,9 +118,6 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
return ret;
ret = ACT_P_CREATED;
} else {
- if (bind)
- return 0;
-
tcf_hash_release(a, bind);
if (!ovr)
return -EEXIST;
--
1.9.1
^ permalink raw reply related
* [net PATCH v2 4/6] net sched: simple action fix late binding
From: Jamal Hadi Salim @ 2016-05-10 20:49 UTC (permalink / raw)
To: davem; +Cc: netdev, xiyou.wangcong, jiri, alexander.duyck, Jamal Hadi Salim
In-Reply-To: <1462913371-9699-1-git-send-email-jhs@emojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
The process below was broken and is fixed with this patch.
//add a simple action and give it an instance id of 1
sudo tc actions add action simple sdata "foobar" index 1
//create a filter which binds to simple action id 1
sudo tc filter add dev $DEV parent ffff: protocol ip prio 1 u32\
match ip dst 17.0.0.1/32 flowid 1:10 action simple index 1
Message before fix was:
RTNETLINK answers: Invalid argument
We have an error talking to the kernel
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/act_simple.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index 75b2be1..3a33fb6 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -87,7 +87,7 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
struct tc_defact *parm;
struct tcf_defact *d;
char *defdata;
- int ret = 0, err;
+ int ret = 0, err, exists = 0;
if (nla == NULL)
return -EINVAL;
@@ -99,13 +99,21 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
if (tb[TCA_DEF_PARMS] == NULL)
return -EINVAL;
- if (tb[TCA_DEF_DATA] == NULL)
- return -EINVAL;
parm = nla_data(tb[TCA_DEF_PARMS]);
+ exists = tcf_hash_check(tn, parm->index, a, bind);
+ if (exists && bind)
+ return 0;
+
+ if (tb[TCA_DEF_DATA] == NULL) {
+ if (exists)
+ tcf_hash_release(a, bind);
+ return -EINVAL;
+ }
+
defdata = nla_data(tb[TCA_DEF_DATA]);
- if (!tcf_hash_check(tn, parm->index, a, bind)) {
+ if (!exists) {
ret = tcf_hash_create(tn, parm->index, est, a,
sizeof(*d), bind, false);
if (ret)
@@ -122,8 +130,6 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
} else {
d = to_defact(a);
- if (bind)
- return 0;
tcf_hash_release(a, bind);
if (!ovr)
return -EEXIST;
--
1.9.1
^ permalink raw reply related
* [net PATCH v2 5/6] net sched: skbedit action fix late binding
From: Jamal Hadi Salim @ 2016-05-10 20:49 UTC (permalink / raw)
To: davem; +Cc: netdev, xiyou.wangcong, jiri, alexander.duyck, Jamal Hadi Salim
In-Reply-To: <1462913371-9699-1-git-send-email-jhs@emojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
The process below was broken and is fixed with this patch.
//add a skbedit action and give it an instance id of 1
sudo tc actions add action skbedit mark 10 index 1
//create a filter which binds to skbedit action id 1
sudo tc filter add dev $DEV parent ffff: protocol ip prio 1 u32\
match ip dst 17.0.0.1/32 flowid 1:10 action skbedit index 1
Message before fix was:
RTNETLINK answers: Invalid argument
We have an error talking to the kernel
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/act_skbedit.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index cfcdbdc..69da5a8 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -69,7 +69,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
struct tcf_skbedit *d;
u32 flags = 0, *priority = NULL, *mark = NULL;
u16 *queue_mapping = NULL;
- int ret = 0, err;
+ int ret = 0, err, exists = 0;
if (nla == NULL)
return -EINVAL;
@@ -96,12 +96,18 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
mark = nla_data(tb[TCA_SKBEDIT_MARK]);
}
- if (!flags)
- return -EINVAL;
-
parm = nla_data(tb[TCA_SKBEDIT_PARMS]);
- if (!tcf_hash_check(tn, parm->index, a, bind)) {
+ exists = tcf_hash_check(tn, parm->index, a, bind);
+ if (exists && bind)
+ return 0;
+
+ if (!flags) {
+ tcf_hash_release(a, bind);
+ return -EINVAL;
+ }
+
+ if (!exists) {
ret = tcf_hash_create(tn, parm->index, est, a,
sizeof(*d), bind, false);
if (ret)
@@ -111,8 +117,6 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
ret = ACT_P_CREATED;
} else {
d = to_skbedit(a);
- if (bind)
- return 0;
tcf_hash_release(a, bind);
if (!ovr)
return -EEXIST;
--
1.9.1
^ permalink raw reply related
* [net PATCH v2 6/6] net sched: ife action fix late binding
From: Jamal Hadi Salim @ 2016-05-10 20:49 UTC (permalink / raw)
To: davem; +Cc: netdev, xiyou.wangcong, jiri, alexander.duyck, Jamal Hadi Salim
In-Reply-To: <1462913371-9699-1-git-send-email-jhs@emojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
The process below was broken and is fixed with this patch.
//add an ife action and give it an instance id of 1
sudo tc actions add action ife encode \
type 0xDEAD allow mark dst 02:15:15:15:15:15 index 1
//create a filter which binds to ife action id 1
sudo tc filter add dev $DEV parent ffff: protocol ip prio 1 u32\
match ip dst 17.0.0.1/32 flowid 1:11 action ife index 1
Message before fix was:
RTNETLINK answers: Invalid argument
We have an error talking to the kernel
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/act_ife.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index c589a9b..343d011 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -423,7 +423,7 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
u16 ife_type = 0;
u8 *daddr = NULL;
u8 *saddr = NULL;
- int ret = 0;
+ int ret = 0, exists = 0;
int err;
err = nla_parse_nested(tb, TCA_IFE_MAX, nla, ife_policy);
@@ -435,25 +435,29 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
parm = nla_data(tb[TCA_IFE_PARMS]);
+ exists = tcf_hash_check(tn, parm->index, a, bind);
+ if (exists && bind)
+ return 0;
+
if (parm->flags & IFE_ENCODE) {
/* Until we get issued the ethertype, we cant have
* a default..
**/
if (!tb[TCA_IFE_TYPE]) {
+ if (exists)
+ tcf_hash_release(a, bind);
pr_info("You MUST pass etherype for encoding\n");
return -EINVAL;
}
}
- if (!tcf_hash_check(tn, parm->index, a, bind)) {
+ if (!exists) {
ret = tcf_hash_create(tn, parm->index, est, a, sizeof(*ife),
bind, false);
if (ret)
return ret;
ret = ACT_P_CREATED;
} else {
- if (bind) /* dont override defaults */
- return 0;
tcf_hash_release(a, bind);
if (!ovr)
return -EEXIST;
@@ -495,6 +499,8 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
NULL);
if (err) {
metadata_parse_err:
+ if (exists)
+ tcf_hash_release(a, bind);
if (ret == ACT_P_CREATED)
_tcf_ife_cleanup(a, bind);
--
1.9.1
^ permalink raw reply related
* Re: [RFC PATCH 0/2] net: threadable napi poll loop
From: Rik van Riel @ 2016-05-10 20:50 UTC (permalink / raw)
To: David Miller, pabeni
Cc: eric.dumazet, netdev, edumazet, jiri, daniel, ast, aduyck, tom,
peterz, mingo, hannes, linux-kernel
In-Reply-To: <20160510.164538.1375529074383780155.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 1439 bytes --]
On Tue, 2016-05-10 at 16:45 -0400, David Miller wrote:
> From: Paolo Abeni <pabeni@redhat.com>
> Date: Tue, 10 May 2016 22:22:50 +0200
>
> > On Tue, 2016-05-10 at 09:08 -0700, Eric Dumazet wrote:
> >> On Tue, 2016-05-10 at 18:03 +0200, Paolo Abeni wrote:
> >>
> >> > If a single core host is under network flood, i.e. ksoftirqd is
> >> > scheduled and it eventually (after processing ~640 packets) will
> let the
> >> > user space process run. The latter will execute a syscall to
> receive a
> >> > packet, which will have to disable/enable bh at least once and
> that will
> >> > cause the processing of another ~640 packets. To receive a
> single packet
> >> > in user space, the kernel has to process more than one thousand
> packets.
> >>
> >> Looks you found the bug then. Have you tried to fix it ?
> ...
> > The ksoftirq and the local_bh_enable() design are the root of the
> > problem, they need to be touched/affected to solve it.
>
> That's not what I read from your description, processing 640 packets
> before going to ksoftirqd seems to the be the absolute root problem.
What would a fix for that look like?
Keep track of the number of processed incoming packets,
and the number of packets handed off, and defer to
ksoftirqd earlier if the statistics suggest packets are
getting dropped on the floor?
Is there a cheap way to do that kind of thing?
--
All Rights Reversed.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 0/2] net: threadable napi poll loop
From: David Miller @ 2016-05-10 20:52 UTC (permalink / raw)
To: riel
Cc: pabeni, eric.dumazet, netdev, edumazet, jiri, daniel, ast, aduyck,
tom, peterz, mingo, hannes, linux-kernel
In-Reply-To: <1462913455.16365.12.camel@redhat.com>
From: Rik van Riel <riel@redhat.com>
Date: Tue, 10 May 2016 16:50:56 -0400
> On Tue, 2016-05-10 at 16:45 -0400, David Miller wrote:
>> From: Paolo Abeni <pabeni@redhat.com>
>> Date: Tue, 10 May 2016 22:22:50 +0200
>>
>> > On Tue, 2016-05-10 at 09:08 -0700, Eric Dumazet wrote:
>> >> On Tue, 2016-05-10 at 18:03 +0200, Paolo Abeni wrote:
>> >>
>> >> > If a single core host is under network flood, i.e. ksoftirqd is
>> >> > scheduled and it eventually (after processing ~640 packets) will
>> let the
>> >> > user space process run. The latter will execute a syscall to
>> receive a
>> >> > packet, which will have to disable/enable bh at least once and
>> that will
>> >> > cause the processing of another ~640 packets. To receive a
>> single packet
>> >> > in user space, the kernel has to process more than one thousand
>> packets.
>> >>
>> >> Looks you found the bug then. Have you tried to fix it ?
>> ...
>> > The ksoftirq and the local_bh_enable() design are the root of the
>> > problem, they need to be touched/affected to solve it.
>>
>> That's not what I read from your description, processing 640 packets
>> before going to ksoftirqd seems to the be the absolute root problem.
>
> What would a fix for that look like?
>
> Keep track of the number of processed incoming packets,
> and the number of packets handed off, and defer to
> ksoftirqd earlier if the statistics suggest packets are
> getting dropped on the floor?
Not by packet count but by something more easily to measure and
scalable to fairness like processing time.
^ permalink raw reply
* Re: [net-next PATCH v2 1/6] net sched: vlan action fix late binding
From: Jamal Hadi Salim @ 2016-05-10 20:52 UTC (permalink / raw)
To: Cong Wang; +Cc: David Miller, Linux Kernel Network Developers
In-Reply-To: <CAM_iQpVtk1AowsH_jDz24yNqAZTCOPtMk=KM0dhFtYUJLfOk4Q@mail.gmail.com>
On 16-05-08 11:08 PM, Cong Wang wrote:
>> + if (aexists)
>> + tcf_hash_release(a, bind);
>
>
> Introduce a goto to reduce duplicated cleanup code?
>
I addressed all your comments except for above goto. There are different
return codes for all failures - and the cleverness of a goto seems
unecessary.
cheers,
jamal
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH] e1000e: prevent division by zero if TIMINCA is zero
From: Jarod Wilson @ 2016-05-10 21:00 UTC (permalink / raw)
To: Rustad, Mark D
Cc: Denys Vlasenko, netdev@vger.kernel.org,
intel-wired-lan@lists.osuosl.org, LKML
In-Reply-To: <5E18E4A4-E10F-46B5-A2F5-FCA10FAA5030@intel.com>
On Fri, May 06, 2016 at 11:43:17PM +0000, Rustad, Mark D wrote:
> Denys Vlasenko <dvlasenk@redhat.com> wrote:
>
> >Users report that under VMWare, er32(TIMINCA) returns zero.
> >This causes division by zero at init time as follows:
> >
> > ==> incvalue = er32(TIMINCA) & E1000_TIMINCA_INCVALUE_MASK;
> > for (i = 0; i < E1000_MAX_82574_SYSTIM_REREADS; i++) {
> > /* latch SYSTIMH on read of SYSTIML */
> > systim_next = (cycle_t)er32(SYSTIML);
> > systim_next |= (cycle_t)er32(SYSTIMH) << 32;
> >
> > time_delta = systim_next - systim;
> > temp = time_delta;
> > ====> rem = do_div(temp, incvalue);
> >
> >This change makes kernel survive this, and users report that
> >NIC does work after this change.
> >
> >Since on real hardware incvalue is never zero, this should not affect
> >real hardware use case.
...
> I seem to recall that this was rejected before because it really is VMWare's
> bug and, if they fix it, any existing VMs that use this will just work.
> Changing the driver will only fix it for vms that install a new driver. I
> don't object to doing it, it just seems like not the most effective place to
> address the issue.
You could also have people who never update VMWare, for whom a kernel
work-around would be better. I think it'd be best to address it both at
the driver level and the emulated hardware level, to improve things for
the most possible users. Those who update neither hypervisor or
kernel/driver, well, they reap what they sow.
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply
* Re: [RFC PATCH 0/2] net: threadable napi poll loop
From: Rik van Riel @ 2016-05-10 21:01 UTC (permalink / raw)
To: David Miller
Cc: pabeni, eric.dumazet, netdev, edumazet, jiri, daniel, ast, aduyck,
tom, peterz, mingo, hannes, linux-kernel
In-Reply-To: <20160510.165215.1602157973141296642.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 1913 bytes --]
On Tue, 2016-05-10 at 16:52 -0400, David Miller wrote:
> From: Rik van Riel <riel@redhat.com>
> Date: Tue, 10 May 2016 16:50:56 -0400
>
> > On Tue, 2016-05-10 at 16:45 -0400, David Miller wrote:
> >> From: Paolo Abeni <pabeni@redhat.com>
> >> Date: Tue, 10 May 2016 22:22:50 +0200
> >>
> >> > On Tue, 2016-05-10 at 09:08 -0700, Eric Dumazet wrote:
> >> >> On Tue, 2016-05-10 at 18:03 +0200, Paolo Abeni wrote:
> >> >>
> >> >> > If a single core host is under network flood, i.e. ksoftirqd
> is
> >> >> > scheduled and it eventually (after processing ~640 packets)
> will
> >> let the
> >> >> > user space process run. The latter will execute a syscall to
> >> receive a
> >> >> > packet, which will have to disable/enable bh at least once
> and
> >> that will
> >> >> > cause the processing of another ~640 packets. To receive a
> >> single packet
> >> >> > in user space, the kernel has to process more than one
> thousand
> >> packets.
> >> >>
> >> >> Looks you found the bug then. Have you tried to fix it ?
> >> ...
> >> > The ksoftirq and the local_bh_enable() design are the root of
> the
> >> > problem, they need to be touched/affected to solve it.
> >>
> >> That's not what I read from your description, processing 640
> packets
> >> before going to ksoftirqd seems to the be the absolute root
> problem.
> >
> > What would a fix for that look like?
> >
> > Keep track of the number of processed incoming packets,
> > and the number of packets handed off, and defer to
> > ksoftirqd earlier if the statistics suggest packets are
> > getting dropped on the floor?
>
> Not by packet count but by something more easily to measure and
> scalable to fairness like processing time.
I need to get back to fixing irq & softirq time accounting,
which does not currently work correctly in all time keeping
modes...
--
All Rights Reversed.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 0/2] net: threadable napi poll loop
From: Eric Dumazet @ 2016-05-10 21:09 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Eric Dumazet, Paolo Abeni, netdev, David S. Miller, Jiri Pirko,
Daniel Borkmann, Alexei Starovoitov, Alexander Duyck, Tom Herbert,
Peter Zijlstra, Ingo Molnar, Rik van Riel, LKML
In-Reply-To: <90f3db8c-c30c-b204-576a-454939ac93ce@stressinduktion.org>
On Tue, May 10, 2016 at 1:46 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> I agree here, but I don't think this patch particularly is a lot of
> bloat and something very interesting people can play with and extend upon.
>
Sure, very rarely patch authors think their stuff is bloat.
I prefer to fix kernel softirq.c, or at least show me that you tried
hard enough.
I am pretty sure that the following would work :
When ksoftirqd is scheduled, remember this in a per cpu variable
(ksoftiqd_scheduled)
When enabling BH , do not call do_softirq() if this variable is set.
ksoftirqd would clear the variable at the right place (probably in
run_ksoftirqd())
Sure, this might add a lot of latency regressions, but lets fix them.
^ permalink raw reply
* Re: [net PATCH v2 1/6] net sched: vlan action fix late binding
From: Cong Wang @ 2016-05-10 21:22 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: David Miller, Linux Kernel Network Developers,
Jiří Pírko, Alexander Duyck
In-Reply-To: <1462913371-9699-2-git-send-email-jhs@emojatatu.com>
On Tue, May 10, 2016 at 1:49 PM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> From: Jamal Hadi Salim <jhs@mojatatu.com>
>
> Late vlan action binding was broken and is fixed with this patch.
>
> //add a vlan action to pop and give it an instance id of 1
> sudo tc actions add action vlan pop index 1
> //create filter which binds to vlan action id 1
> sudo tc filter add dev $DEV parent ffff: protocol ip prio 1 u32 \
> match ip dst 17.0.0.1/32 flowid 1:1 action vlan index 1
>
> current message(before bug fix) was:
> RTNETLINK answers: Invalid argument
> We have an error talking to the kernel
>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Cong Wang <xiyou.wangcong@gmail.com>
^ permalink raw reply
* Re: [net PATCH v2 2/6] net sched: ipt action fix late binding
From: Cong Wang @ 2016-05-10 21:24 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: David Miller, Linux Kernel Network Developers,
Jiří Pírko, Alexander Duyck
In-Reply-To: <1462913371-9699-3-git-send-email-jhs@emojatatu.com>
On Tue, May 10, 2016 at 1:49 PM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> From: Jamal Hadi Salim <jhs@mojatatu.com>
>
> This was broken and is fixed with this patch.
>
> //add an ipt action and give it an instance id of 1
> sudo tc actions add action ipt -j mark --set-mark 2 index 1
> //create a filter which binds to ipt action id 1
> sudo tc filter add dev $DEV parent ffff: protocol ip prio 1 u32\
> match ip dst 17.0.0.1/32 flowid 1:10 action ipt index 1
>
> Message before bug fix was:
> RTNETLINK answers: Invalid argument
> We have an error talking to the kernel
>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Cong Wang <xiyou.wangcong@gmail.com>
^ permalink raw reply
* Re: [net PATCH v2 3/6] net sched: mirred action fix late binding
From: Cong Wang @ 2016-05-10 21:25 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: David Miller, Linux Kernel Network Developers,
Jiří Pírko, Alexander Duyck
In-Reply-To: <1462913371-9699-4-git-send-email-jhs@emojatatu.com>
On Tue, May 10, 2016 at 1:49 PM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> From: Jamal Hadi Salim <jhs@mojatatu.com>
>
> The process below was broken and is fixed with this patch.
>
> //add an mirred action and give it an instance id of 1
> sudo tc actions add action mirred egress mirror dev $MDEV index 1
> //create a filter which binds to mirred action id 1
> sudo tc filter add dev $DEV parent ffff: protocol ip prio 1 u32\
> match ip dst 17.0.0.1/32 flowid 1:10 action mirred index 1
>
> Message before bug fix was:
> RTNETLINK answers: Invalid argument
> We have an error talking to the kernel
>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Cong Wang <xiyou.wangcong@gmail.com>
^ 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