* [PATCH v2 net-next 0/6] tcp: implement SACK compression
From: Eric Dumazet @ 2018-05-17 17:47 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Toke Høiland-Jørgensen, Neal Cardwell,
Yuchung Cheng, Soheil Hassas Yeganeh, Eric Dumazet, Eric Dumazet
When TCP receives an out-of-order packet, it immediately sends
a SACK packet, generating network load but also forcing the
receiver to send 1-MSS pathological packets, increasing its
RTX queue length/depth, and thus processing time.
Wifi networks suffer from this aggressive behavior, but generally
speaking, all these SACK packets add fuel to the fire when networks
are under congestion.
This patch series adds SACK compression, but the infrastructure
could be leveraged to also compress ACK in the future.
v2: Addressed Neal feedback.
Added two sysctls to allow fine tuning, or even disabling the feature.
Eric Dumazet (6):
tcp: use __sock_put() instead of sock_put() in tcp_clear_xmit_timers()
tcp: do not force quickack when receiving out-of-order packets
tcp: add SACK compression
tcp: add TCPAckCompressed SNMP counter
tcp: add tcp_comp_sack_delay_ns sysctl
tcp: add tcp_comp_sack_nr sysctl
Documentation/networking/ip-sysctl.txt | 13 ++++++++++
include/linux/tcp.h | 2 ++
include/net/netns/ipv4.h | 2 ++
include/net/tcp.h | 5 +++-
include/uapi/linux/snmp.h | 1 +
net/ipv4/proc.c | 1 +
net/ipv4/sysctl_net_ipv4.c | 17 +++++++++++++
net/ipv4/tcp.c | 1 +
net/ipv4/tcp_input.c | 34 ++++++++++++++++++++------
net/ipv4/tcp_ipv4.c | 2 ++
net/ipv4/tcp_output.c | 9 +++++++
net/ipv4/tcp_timer.c | 25 +++++++++++++++++++
12 files changed, 103 insertions(+), 9 deletions(-)
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply
* [PATCH v2 net-next 1/6] tcp: use __sock_put() instead of sock_put() in tcp_clear_xmit_timers()
From: Eric Dumazet @ 2018-05-17 17:47 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Toke Høiland-Jørgensen, Neal Cardwell,
Yuchung Cheng, Soheil Hassas Yeganeh, Eric Dumazet, Eric Dumazet
In-Reply-To: <20180517174739.192489-1-edumazet@google.com>
Socket can not disappear under us.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
---
include/net/tcp.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index a08eab58ef7001b3e141e3722fd8a3875e5c5d7d..6ffc8bd894876ad23407f5ec4994350139af85e7 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -558,7 +558,7 @@ void tcp_init_xmit_timers(struct sock *);
static inline void tcp_clear_xmit_timers(struct sock *sk)
{
if (hrtimer_try_to_cancel(&tcp_sk(sk)->pacing_timer) == 1)
- sock_put(sk);
+ __sock_put(sk);
inet_csk_clear_xmit_timers(sk);
}
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
* [PATCH v2 net-next 2/6] tcp: do not force quickack when receiving out-of-order packets
From: Eric Dumazet @ 2018-05-17 17:47 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Toke Høiland-Jørgensen, Neal Cardwell,
Yuchung Cheng, Soheil Hassas Yeganeh, Eric Dumazet, Eric Dumazet
In-Reply-To: <20180517174739.192489-1-edumazet@google.com>
As explained in commit 9f9843a751d0 ("tcp: properly handle stretch
acks in slow start"), TCP stacks have to consider how many packets
are acknowledged in one single ACK, because of GRO, but also
because of ACK compression or losses.
We plan to add SACK compression in the following patch, we
must therefore not call tcp_enter_quickack_mode()
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
---
net/ipv4/tcp_input.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index b188e0d75edd9e5e1c9f0355818caa932fef7416..99fcab7e6570c8b8758ea4b15cdd26df29fb4fd6 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4708,8 +4708,6 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
if (!before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt + tcp_receive_window(tp)))
goto out_of_window;
- tcp_enter_quickack_mode(sk);
-
if (before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
/* Partial packet, seq < rcv_next < end_seq */
SOCK_DEBUG(sk, "partial packet: rcv_next %X seq %X - %X\n",
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
* [PATCH v2 net-next 3/6] tcp: add SACK compression
From: Eric Dumazet @ 2018-05-17 17:47 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Toke Høiland-Jørgensen, Neal Cardwell,
Yuchung Cheng, Soheil Hassas Yeganeh, Eric Dumazet, Eric Dumazet
In-Reply-To: <20180517174739.192489-1-edumazet@google.com>
When TCP receives an out-of-order packet, it immediately sends
a SACK packet, generating network load but also forcing the
receiver to send 1-MSS pathological packets, increasing its
RTX queue length/depth, and thus processing time.
Wifi networks suffer from this aggressive behavior, but generally
speaking, all these SACK packets add fuel to the fire when networks
are under congestion.
This patch adds a high resolution timer and tp->compressed_ack counter.
Instead of sending a SACK, we program this timer with a small delay,
based on SRTT and capped to 1 ms :
delay = min ( 5 % of SRTT, 1 ms)
If subsequent SACKs need to be sent while the timer has not yet
expired, we simply increment tp->compressed_ack.
When timer expires, a SACK is sent with the latest information.
Whenever an ACK is sent (if data is sent, or if in-order
data is received) timer is canceled.
Note that tcp_sack_new_ofo_skb() is able to force a SACK to be sent
if the sack blocks need to be shuffled, even if the timer has not
expired.
A new SNMP counter is added in the following patch.
Two other patches add sysctls to allow changing the 1,000,000 and 44
values that this commit hard-coded.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/tcp.h | 2 ++
include/net/tcp.h | 3 +++
net/ipv4/tcp.c | 1 +
net/ipv4/tcp_input.c | 31 +++++++++++++++++++++++++------
net/ipv4/tcp_output.c | 7 +++++++
net/ipv4/tcp_timer.c | 25 +++++++++++++++++++++++++
6 files changed, 63 insertions(+), 6 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 807776928cb8610fe97121fbc3c600b08d5d2991..72705eaf4b84060a45bf04d5170f389a18010eac 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -218,6 +218,7 @@ struct tcp_sock {
reord:1; /* reordering detected */
} rack;
u16 advmss; /* Advertised MSS */
+ u8 compressed_ack;
u32 chrono_start; /* Start time in jiffies of a TCP chrono */
u32 chrono_stat[3]; /* Time in jiffies for chrono_stat stats */
u8 chrono_type:2, /* current chronograph type */
@@ -297,6 +298,7 @@ struct tcp_sock {
u32 sacked_out; /* SACK'd packets */
struct hrtimer pacing_timer;
+ struct hrtimer compressed_ack_timer;
/* from STCP, retrans queue hinting */
struct sk_buff* lost_skb_hint;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 6ffc8bd894876ad23407f5ec4994350139af85e7..c8c65ae62955eb12a9a6489fa8e008fd89f89f16 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -560,6 +560,9 @@ static inline void tcp_clear_xmit_timers(struct sock *sk)
if (hrtimer_try_to_cancel(&tcp_sk(sk)->pacing_timer) == 1)
__sock_put(sk);
+ if (hrtimer_try_to_cancel(&tcp_sk(sk)->compressed_ack_timer) == 1)
+ __sock_put(sk);
+
inet_csk_clear_xmit_timers(sk);
}
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 62b776f9003798eaf06992a4eb0914d17646aa61..0a2ea0bbf867271db05aedd7d48b193677664321 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2595,6 +2595,7 @@ int tcp_disconnect(struct sock *sk, int flags)
dst_release(sk->sk_rx_dst);
sk->sk_rx_dst = NULL;
tcp_saved_syn_free(tp);
+ tp->compressed_ack = 0;
/* Clean up fastopen related fields */
tcp_free_fastopen_req(tp);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 99fcab7e6570c8b8758ea4b15cdd26df29fb4fd6..2e03c1b4d327558fa4187b9dd53432df2e7d307f 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4242,6 +4242,8 @@ static void tcp_sack_new_ofo_skb(struct sock *sk, u32 seq, u32 end_seq)
* If the sack array is full, forget about the last one.
*/
if (this_sack >= TCP_NUM_SACKS) {
+ if (tp->compressed_ack)
+ tcp_send_ack(sk);
this_sack--;
tp->rx_opt.num_sacks--;
sp--;
@@ -5074,6 +5076,7 @@ static inline void tcp_data_snd_check(struct sock *sk)
static void __tcp_ack_snd_check(struct sock *sk, int ofo_possible)
{
struct tcp_sock *tp = tcp_sk(sk);
+ unsigned long delay;
/* More than one full frame received... */
if (((tp->rcv_nxt - tp->rcv_wup) > inet_csk(sk)->icsk_ack.rcv_mss &&
@@ -5085,15 +5088,31 @@ static void __tcp_ack_snd_check(struct sock *sk, int ofo_possible)
(tp->rcv_nxt - tp->copied_seq < sk->sk_rcvlowat ||
__tcp_select_window(sk) >= tp->rcv_wnd)) ||
/* We ACK each frame or... */
- tcp_in_quickack_mode(sk) ||
- /* We have out of order data. */
- (ofo_possible && !RB_EMPTY_ROOT(&tp->out_of_order_queue))) {
- /* Then ack it now */
+ tcp_in_quickack_mode(sk)) {
+send_now:
tcp_send_ack(sk);
- } else {
- /* Else, send delayed ack. */
+ return;
+ }
+
+ if (!ofo_possible || RB_EMPTY_ROOT(&tp->out_of_order_queue)) {
tcp_send_delayed_ack(sk);
+ return;
}
+
+ if (!tcp_is_sack(tp) || tp->compressed_ack >= 44)
+ goto send_now;
+ tp->compressed_ack++;
+
+ if (hrtimer_is_queued(&tp->compressed_ack_timer))
+ return;
+
+ /* compress ack timer : 5 % of srtt, but no more than 1 ms */
+
+ delay = min_t(unsigned long, NSEC_PER_MSEC,
+ tp->rcv_rtt_est.rtt_us * (NSEC_PER_USEC >> 3)/20);
+ sock_hold(sk);
+ hrtimer_start(&tp->compressed_ack_timer, ns_to_ktime(delay),
+ HRTIMER_MODE_REL_PINNED_SOFT);
}
static inline void tcp_ack_snd_check(struct sock *sk)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 0d8f950a9006598c70dbf51e281a3fe32dfaa234..7ee98aad82b758674ca7f3e90bd3fc165e8fcd45 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -162,6 +162,13 @@ static void tcp_event_data_sent(struct tcp_sock *tp,
/* Account for an ACK we sent. */
static inline void tcp_event_ack_sent(struct sock *sk, unsigned int pkts)
{
+ struct tcp_sock *tp = tcp_sk(sk);
+
+ if (unlikely(tp->compressed_ack)) {
+ tp->compressed_ack = 0;
+ if (hrtimer_try_to_cancel(&tp->compressed_ack_timer) == 1)
+ __sock_put(sk);
+ }
tcp_dec_quickack_mode(sk, pkts);
inet_csk_clear_xmit_timer(sk, ICSK_TIME_DACK);
}
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 92bdf64fffae3a5be291ca419eb21276b4c8cbae..3b3611729928f77934e0298bb248e55c7a7c5def 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -708,6 +708,27 @@ static void tcp_keepalive_timer (struct timer_list *t)
sock_put(sk);
}
+static enum hrtimer_restart tcp_compressed_ack_kick(struct hrtimer *timer)
+{
+ struct tcp_sock *tp = container_of(timer, struct tcp_sock, compressed_ack_timer);
+ struct sock *sk = (struct sock *)tp;
+
+ bh_lock_sock(sk);
+ if (!sock_owned_by_user(sk)) {
+ if (tp->compressed_ack)
+ tcp_send_ack(sk);
+ } else {
+ if (!test_and_set_bit(TCP_DELACK_TIMER_DEFERRED,
+ &sk->sk_tsq_flags))
+ sock_hold(sk);
+ }
+ bh_unlock_sock(sk);
+
+ sock_put(sk);
+
+ return HRTIMER_NORESTART;
+}
+
void tcp_init_xmit_timers(struct sock *sk)
{
inet_csk_init_xmit_timers(sk, &tcp_write_timer, &tcp_delack_timer,
@@ -715,4 +736,8 @@ void tcp_init_xmit_timers(struct sock *sk)
hrtimer_init(&tcp_sk(sk)->pacing_timer, CLOCK_MONOTONIC,
HRTIMER_MODE_ABS_PINNED_SOFT);
tcp_sk(sk)->pacing_timer.function = tcp_pace_kick;
+
+ hrtimer_init(&tcp_sk(sk)->compressed_ack_timer, CLOCK_MONOTONIC,
+ HRTIMER_MODE_REL_PINNED_SOFT);
+ tcp_sk(sk)->compressed_ack_timer.function = tcp_compressed_ack_kick;
}
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
* [PATCH v2 net-next 4/6] tcp: add TCPAckCompressed SNMP counter
From: Eric Dumazet @ 2018-05-17 17:47 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Toke Høiland-Jørgensen, Neal Cardwell,
Yuchung Cheng, Soheil Hassas Yeganeh, Eric Dumazet, Eric Dumazet
In-Reply-To: <20180517174739.192489-1-edumazet@google.com>
This counter tracks number of ACK packets that the host has not sent,
thanks to ACK compression.
Sample output :
$ nstat -n;sleep 1;nstat|egrep "IpInReceives|IpOutRequests|TcpInSegs|TcpOutSegs|TcpExtTCPAckCompressed"
IpInReceives 123250 0.0
IpOutRequests 3684 0.0
TcpInSegs 123251 0.0
TcpOutSegs 3684 0.0
TcpExtTCPAckCompressed 119252 0.0
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
---
include/uapi/linux/snmp.h | 1 +
net/ipv4/proc.c | 1 +
net/ipv4/tcp_output.c | 2 ++
3 files changed, 4 insertions(+)
diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
index d02e859301ff499dd72a1c0e1b56bed10a9397a6..750d89120335eb489f698191edb6c5110969fa8c 100644
--- a/include/uapi/linux/snmp.h
+++ b/include/uapi/linux/snmp.h
@@ -278,6 +278,7 @@ enum
LINUX_MIB_TCPMTUPSUCCESS, /* TCPMTUPSuccess */
LINUX_MIB_TCPDELIVERED, /* TCPDelivered */
LINUX_MIB_TCPDELIVEREDCE, /* TCPDeliveredCE */
+ LINUX_MIB_TCPACKCOMPRESSED, /* TCPAckCompressed */
__LINUX_MIB_MAX
};
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 261b71d0ccc5c17c6032bf67eb8f842006766e64..6c1ff89a60fa0a3485dcc71fafc799e798d5dc11 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -298,6 +298,7 @@ static const struct snmp_mib snmp4_net_list[] = {
SNMP_MIB_ITEM("TCPMTUPSuccess", LINUX_MIB_TCPMTUPSUCCESS),
SNMP_MIB_ITEM("TCPDelivered", LINUX_MIB_TCPDELIVERED),
SNMP_MIB_ITEM("TCPDeliveredCE", LINUX_MIB_TCPDELIVEREDCE),
+ SNMP_MIB_ITEM("TCPAckCompressed", LINUX_MIB_TCPACKCOMPRESSED),
SNMP_MIB_SENTINEL
};
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 7ee98aad82b758674ca7f3e90bd3fc165e8fcd45..437bb7ceba7fd388abac1c12f2920b02be77bad9 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -165,6 +165,8 @@ static inline void tcp_event_ack_sent(struct sock *sk, unsigned int pkts)
struct tcp_sock *tp = tcp_sk(sk);
if (unlikely(tp->compressed_ack)) {
+ NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPACKCOMPRESSED,
+ tp->compressed_ack);
tp->compressed_ack = 0;
if (hrtimer_try_to_cancel(&tp->compressed_ack_timer) == 1)
__sock_put(sk);
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
* [PATCH v2 net-next 5/6] tcp: add tcp_comp_sack_delay_ns sysctl
From: Eric Dumazet @ 2018-05-17 17:47 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Toke Høiland-Jørgensen, Neal Cardwell,
Yuchung Cheng, Soheil Hassas Yeganeh, Eric Dumazet, Eric Dumazet
In-Reply-To: <20180517174739.192489-1-edumazet@google.com>
This per netns sysctl allows for TCP SACK compression fine-tuning.
Its default value is 1,000,000, or 1 ms to meet TSO autosizing period.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
Documentation/networking/ip-sysctl.txt | 7 +++++++
include/net/netns/ipv4.h | 1 +
net/ipv4/sysctl_net_ipv4.c | 7 +++++++
net/ipv4/tcp_input.c | 4 ++--
net/ipv4/tcp_ipv4.c | 1 +
5 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 59afc9a10b4f42bac6871f9183f2bac78ca87803..a7d44627d6356171891fb3c0ab5783f69b6dcb3d 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -523,6 +523,13 @@ tcp_rmem - vector of 3 INTEGERs: min, default, max
tcp_sack - BOOLEAN
Enable select acknowledgments (SACKS).
+tcp_comp_sack_delay_ns - LONG INTEGER
+ TCP tries to reduce number of SACK sent, using a timer
+ based on 5% of SRTT, capped by this sysctl, in nano seconds.
+ The default is 1ms, based on TSO autosizing period.
+
+ Default : 1,000,000 ns (1 ms)
+
tcp_slow_start_after_idle - BOOLEAN
If set, provide RFC2861 behavior and time out the congestion
window after an idle period. An idle period is defined at
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 8491bc9c86b1553ab603e4363e8e38ca7ff547e0..927318243cfaa2ddd8eb423c6ba6e66253f771d3 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -160,6 +160,7 @@ struct netns_ipv4 {
int sysctl_tcp_pacing_ca_ratio;
int sysctl_tcp_wmem[3];
int sysctl_tcp_rmem[3];
+ unsigned long sysctl_tcp_comp_sack_delay_ns;
struct inet_timewait_death_row tcp_death_row;
int sysctl_max_syn_backlog;
int sysctl_tcp_fastopen;
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 4b195bac8ac0eefe0a224528ad854338c4f8e6e3..11fbfdc1566eca95f91360522178295318277588 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -1151,6 +1151,13 @@ static struct ctl_table ipv4_net_table[] = {
.proc_handler = proc_dointvec_minmax,
.extra1 = &one,
},
+ {
+ .procname = "tcp_comp_sack_delay_ns",
+ .data = &init_net.ipv4.sysctl_tcp_comp_sack_delay_ns,
+ .maxlen = sizeof(unsigned long),
+ .mode = 0644,
+ .proc_handler = proc_doulongvec_minmax,
+ },
{
.procname = "udp_rmem_min",
.data = &init_net.ipv4.sysctl_udp_rmem_min,
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 2e03c1b4d327558fa4187b9dd53432df2e7d307f..480a647fd5922c1736455e625890da5f85d19ea7 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5106,9 +5106,9 @@ static void __tcp_ack_snd_check(struct sock *sk, int ofo_possible)
if (hrtimer_is_queued(&tp->compressed_ack_timer))
return;
- /* compress ack timer : 5 % of srtt, but no more than 1 ms */
+ /* 5 % of srtt, but no more than tcp_comp_sack_delay_ns */
- delay = min_t(unsigned long, NSEC_PER_MSEC,
+ delay = min_t(unsigned long, sock_net(sk)->ipv4.sysctl_tcp_comp_sack_delay_ns,
tp->rcv_rtt_est.rtt_us * (NSEC_PER_USEC >> 3)/20);
sock_hold(sk);
hrtimer_start(&tp->compressed_ack_timer, ns_to_ktime(delay),
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index caf23de88f8a369c2038cecd34ce42c522487e90..a3f4647341db2eb5a63c3e9f1e8b93099aedadab 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2572,6 +2572,7 @@ static int __net_init tcp_sk_init(struct net *net)
init_net.ipv4.sysctl_tcp_wmem,
sizeof(init_net.ipv4.sysctl_tcp_wmem));
}
+ net->ipv4.sysctl_tcp_comp_sack_delay_ns = NSEC_PER_MSEC;
net->ipv4.sysctl_tcp_fastopen = TFO_CLIENT_ENABLE;
spin_lock_init(&net->ipv4.tcp_fastopen_ctx_lock);
net->ipv4.sysctl_tcp_fastopen_blackhole_timeout = 60 * 60;
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
* Re: [PATCH] net: ethernet: ti: cpsw: fix packet leaking in dual_mac mode
From: Naresh Kamboju @ 2018-05-17 17:48 UTC (permalink / raw)
To: David Miller
Cc: Grygorii Strashko, netdev, nsekhar, open list, linux-omap,
Greg Kroah-Hartman
In-Reply-To: <20180502.110841.378324128079268584.davem@davemloft.net>
On 2 May 2018 at 20:38, David Miller <davem@davemloft.net> wrote:
> From: Grygorii Strashko <grygorii.strashko@ti.com>
> Date: Tue, 1 May 2018 12:41:22 -0500
<trim>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>
> Applied and queued up for -stable, thank you.
4.4 stable-rc build failed for arm32.
MACHINE=am57xx-evm
Build error log:
--------------------
drivers/net/ethernet/ti/cpsw.c:
In function 'cpsw_add_dual_emac_def_ale_entries':
drivers/net/ethernet/ti/cpsw.c:1112:23:
error: 'cpsw' undeclared (first use in this function)
cpsw_ale_control_set(cpsw->ale, slave_port,
^~~~
drivers/net/ethernet/ti/cpsw.c:1112:23: note:
each undeclared identifier is reported only once for each function it appears
in
scripts/Makefile.build:269: recipe for target 'drivers/net/ethernet/ti/cpsw.o'
failed
make[6]: *** [drivers/net/ethernet/ti/cpsw.o] Error 1
scripts/Makefile.build:476: recipe for target 'drivers/net/ethernet/ti' failed
make[5]: *** [drivers/net/ethernet/ti] Error 2
Complete log link:
https://ci.linaro.org/job/openembedded-lkft-linux-stable-rc-4.4/DISTRO=rpb,MACHINE=am57xx-evm,label=docker-lkft/205/console
^ permalink raw reply
* [PATCH v2 net-next 6/6] tcp: add tcp_comp_sack_nr sysctl
From: Eric Dumazet @ 2018-05-17 17:47 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Toke Høiland-Jørgensen, Neal Cardwell,
Yuchung Cheng, Soheil Hassas Yeganeh, Eric Dumazet, Eric Dumazet
In-Reply-To: <20180517174739.192489-1-edumazet@google.com>
This per netns sysctl allows for TCP SACK compression fine-tuning.
This limits number of SACK that can be compressed.
Using 0 disables SACK compression.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
Documentation/networking/ip-sysctl.txt | 6 ++++++
include/net/netns/ipv4.h | 1 +
net/ipv4/sysctl_net_ipv4.c | 10 ++++++++++
net/ipv4/tcp_input.c | 3 ++-
net/ipv4/tcp_ipv4.c | 1 +
5 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index a7d44627d6356171891fb3c0ab5783f69b6dcb3d..e46fe037da48ace9bb03bb61690fb238601c3558 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -530,6 +530,12 @@ tcp_comp_sack_delay_ns - LONG INTEGER
Default : 1,000,000 ns (1 ms)
+tcp_comp_sack_nr - INTEGER
+ Max numer of SACK that can be compressed.
+ Using 0 disables SACK compression.
+
+ Detault : 44
+
tcp_slow_start_after_idle - BOOLEAN
If set, provide RFC2861 behavior and time out the congestion
window after an idle period. An idle period is defined at
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 927318243cfaa2ddd8eb423c6ba6e66253f771d3..661348f23ea5a3a9320b2cafcd17e23960214771 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -160,6 +160,7 @@ struct netns_ipv4 {
int sysctl_tcp_pacing_ca_ratio;
int sysctl_tcp_wmem[3];
int sysctl_tcp_rmem[3];
+ int sysctl_tcp_comp_sack_nr;
unsigned long sysctl_tcp_comp_sack_delay_ns;
struct inet_timewait_death_row tcp_death_row;
int sysctl_max_syn_backlog;
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 11fbfdc1566eca95f91360522178295318277588..d2eed3ddcb0a1ad9778d96d46c685f6c60b93d8d 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -46,6 +46,7 @@ static int tcp_syn_retries_min = 1;
static int tcp_syn_retries_max = MAX_TCP_SYNCNT;
static int ip_ping_group_range_min[] = { 0, 0 };
static int ip_ping_group_range_max[] = { GID_T_MAX, GID_T_MAX };
+static int comp_sack_nr_max = 255;
/* obsolete */
static int sysctl_tcp_low_latency __read_mostly;
@@ -1158,6 +1159,15 @@ static struct ctl_table ipv4_net_table[] = {
.mode = 0644,
.proc_handler = proc_doulongvec_minmax,
},
+ {
+ .procname = "tcp_comp_sack_nr",
+ .data = &init_net.ipv4.sysctl_tcp_comp_sack_nr,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = &zero,
+ .extra2 = &comp_sack_nr_max,
+ },
{
.procname = "udp_rmem_min",
.data = &init_net.ipv4.sysctl_udp_rmem_min,
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 480a647fd5922c1736455e625890da5f85d19ea7..f763637e83c7412da73cea77b0eae4076e90ca6a 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5099,7 +5099,8 @@ static void __tcp_ack_snd_check(struct sock *sk, int ofo_possible)
return;
}
- if (!tcp_is_sack(tp) || tp->compressed_ack >= 44)
+ if (!tcp_is_sack(tp) ||
+ tp->compressed_ack >= sock_net(sk)->ipv4.sysctl_tcp_comp_sack_nr)
goto send_now;
tp->compressed_ack++;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index a3f4647341db2eb5a63c3e9f1e8b93099aedadab..adbdb503db0c983ef4185f83b138aa51bafd15bf 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2573,6 +2573,7 @@ static int __net_init tcp_sk_init(struct net *net)
sizeof(init_net.ipv4.sysctl_tcp_wmem));
}
net->ipv4.sysctl_tcp_comp_sack_delay_ns = NSEC_PER_MSEC;
+ net->ipv4.sysctl_tcp_comp_sack_nr = 44;
net->ipv4.sysctl_tcp_fastopen = TFO_CLIENT_ENABLE;
spin_lock_init(&net->ipv4.tcp_fastopen_ctx_lock);
net->ipv4.sysctl_tcp_fastopen_blackhole_timeout = 60 * 60;
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
* Re: [patch net-next] nfp: flower: set sysfs link to device for representors
From: Jakub Kicinski @ 2018-05-17 17:51 UTC (permalink / raw)
To: Jiri Pirko, Or Gerlitz
Cc: Linux Netdev List, David Miller, Simon Horman, Dirk van der Merwe,
John Hurley, Pieter Jansen van Vuuren, oss-drivers, Jan Gutter
In-Reply-To: <20180517124210.GS1972@nanopsycho>
On Thu, 17 May 2018 14:42:10 +0200, Jiri Pirko wrote:
> Thu, May 17, 2018 at 02:25:14PM CEST, gerlitz.or@gmail.com wrote:
> >On Thu, May 17, 2018 at 1:05 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> >> From: Jiri Pirko <jiri@mellanox.com>
> >>
> >> Do this so the sysfs has "device" link correctly set.
> >
> >please no
> >
> >This is likely to create bunch of issues with respect to how libvirt
> >deals with the representors.
>
> Once netdev is there because of some probed pci device, this link should
> be set. That is one of the basics.
>
> What "bunch of issues" are you talking about? Please be specific.
Libvirt may pick any device under sys/bus/pci/devices/$vf/physfn/net/
to run legacy SR-IOV NDOs on. nfp currently exposes those on all
representors.
Or correctly points out that running SR-IOV NDOs on non-PF reprs makes
very limited amount of sense and we should fix libvirt and remove the
NDOs for non-PF nfp reprs.
AFAICT since nfp does provide the NDOs on all reprs nothing should
break with this patch, but ip link will now start listing VF state on
each representor. Meaning that if we have 100 VFs enabled simple "ip
link list" will produce upwards of 10,000 lines of output.
^ permalink raw reply
* Re: [PATCH bpf-next 2/7] bpf: introduce bpf subcommand BPF_PERF_EVENT_QUERY
From: Yonghong Song @ 2018-05-17 17:50 UTC (permalink / raw)
To: Daniel Borkmann, Peter Zijlstra; +Cc: ast, netdev, kernel-team
In-Reply-To: <c9846f8a-1bc6-0fc2-c1e6-23f21401f278@iogearbox.net>
On 5/17/18 8:32 AM, Daniel Borkmann wrote:
> On 05/16/2018 11:59 PM, Yonghong Song wrote:
>> On 5/16/18 4:27 AM, Peter Zijlstra wrote:
>>> On Tue, May 15, 2018 at 04:45:16PM -0700, Yonghong Song wrote:
>>>> Currently, suppose a userspace application has loaded a bpf program
>>>> and attached it to a tracepoint/kprobe/uprobe, and a bpf
>>>> introspection tool, e.g., bpftool, wants to show which bpf program
>>>> is attached to which tracepoint/kprobe/uprobe. Such attachment
>>>> information will be really useful to understand the overall bpf
>>>> deployment in the system.
>>>>
>>>> There is a name field (16 bytes) for each program, which could
>>>> be used to encode the attachment point. There are some drawbacks
>>>> for this approaches. First, bpftool user (e.g., an admin) may not
>>>> really understand the association between the name and the
>>>> attachment point. Second, if one program is attached to multiple
>>>> places, encoding a proper name which can imply all these
>>>> attachments becomes difficult.
>>>>
>>>> This patch introduces a new bpf subcommand BPF_PERF_EVENT_QUERY.
>>>> Given a pid and fd, if the <pid, fd> is associated with a
>>>> tracepoint/kprobe/uprobea perf event, BPF_PERF_EVENT_QUERY will return
>>>> . prog_id
>>>> . tracepoint name, or
>>>> . k[ret]probe funcname + offset or kernel addr, or
>>>> . u[ret]probe filename + offset
>>>> to the userspace.
>>>> The user can use "bpftool prog" to find more information about
>>>> bpf program itself with prog_id.
>>>>
>>>> Signed-off-by: Yonghong Song <yhs@fb.com>
>>>> ---
>>>> include/linux/trace_events.h | 15 ++++++
>>>> include/uapi/linux/bpf.h | 25 ++++++++++
>>>> kernel/bpf/syscall.c | 113 +++++++++++++++++++++++++++++++++++++++++++
>>>> kernel/trace/bpf_trace.c | 53 ++++++++++++++++++++
>>>> kernel/trace/trace_kprobe.c | 29 +++++++++++
>>>> kernel/trace/trace_uprobe.c | 22 +++++++++
>>>> 6 files changed, 257 insertions(+)
>>>
>>> Why is the command called *_PERF_EVENT_* ? Are there not a lot of !perf
>>> places to attach BPF proglets?
>>
>> Just gave a complete picture, the below are major places to attach
>> BPF programs:
>> . perf based (through perf ioctl)
>> . raw tracepoint based (through bpf interface)
>>
>> . netlink interface for tc, xdp, tunneling
>> . setsockopt for socket filters
>> . cgroup based (bpf attachment subcommand)
>> mostly networking and io devices
>> . some other networking socket related (sk_skb stream/parser/verdict,
>> sk_msg verdict) through bpf attachment subcommand.
>>
>> Currently, for cgroup based attachment, we have BPF_PROG_QUERY with input cgroup file descriptor. For other networking based queries, we
>> may need to enumerate tc filters, networking devices, open sockets, etc.
>> to get the attachment information.
>>
>> So to have one BPF_QUERY command line may be too complex to
>> cover all cases.
>>
>> But you are right that BPF_PERF_EVENT_QUERY name is too narrow since
>> it should be used for other (pid, fd) based queries as well (e.g., socket, or other potential uses in the future).
>>
>> How about the subcommand name BPF_TASK_FD_QUERY and make bpf_attr.task_fd_query extensible?
>
> I like the introspection output it provides in 7/7, it's really great!
> So the query interface would only ever be tied to BPF progs whose attach
> life time is tied to the life time of the application and as soon as all
> refs on the fd are released it's unloaded from the system. BPF_TASK_FD_QUERY
> seems okay to me, or something like BPF_ATTACH_QUERY. Even if the name is
> slightly more generic, it might be more fitting with other cmds like
> BPF_PROG_QUERY we have where we tell an attach point to retrieve all progs
> from it (though only tied to cgroups right now, it may not be in future).
I think BPF_TASK_FD_QUERY is okay. Using BPF_ATTACH_QUERY indeed seems
a little bit broader to me as other query subcommands are possible to
query attachments with different input.
BPF_PROG_QUERY is also trying to query attachment. Currently, given a
cgroup fd, it will query prog array attached. Sean has the patch to
attach bpf programs to a RC device, and given a device fd, it will
query prog array attached to that device.
>
> For all the others that are not strictly tied to the task but global, bpftool
> would then need to be extended to query the various other interfaces like
> netlink for retrieval which is on todo for some point in future as well. So
> this set nicely complements this introspection aspect.
Totally agree.
Thanks!
>
> Thanks,
> Daniel
>
^ permalink raw reply
* Re: [patch net-next] nfp: flower: fix error path during representor creation
From: Jakub Kicinski @ 2018-05-17 17:56 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, simon.horman, dirk.vandermerwe, john.hurley,
pieter.jansenvanvuuren, oss-drivers
In-Reply-To: <20180517100643.24044-1-jiri@resnulli.us>
On Thu, 17 May 2018 12:06:43 +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Don't store repr pointer to reprs array until the representor is
> successfully created. This avoids message about "representor
> destruction" even when it was never created. Also it cleans-up the flow.
> Also, check return value after port alloc.
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Thank you!
^ permalink raw reply
* KASAN: use-after-free Read in timer_is_static_object
From: syzbot @ 2018-05-17 17:57 UTC (permalink / raw)
To: alexey.kodanev, davem, dccp, edumazet, gerrit, keescook,
linux-kernel, netdev, soheil, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: e6506eb24187 Merge tag 'trace-v4.17-rc4-2' of git://git.ke..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=177fe477800000
kernel config: https://syzkaller.appspot.com/x/.config?x=f3b4e30da84ec1ed
dashboard link: https://syzkaller.appspot.com/bug?extid=5d47e9ec91a6f15dbd6f
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+5d47e9ec91a6f15dbd6f@syzkaller.appspotmail.com
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000016
RBP: 000000000072bea0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000017
R13: 0000000000000053 R14: 00000000006f4868 R15: 0000000000000001
==================================================================
BUG: KASAN: use-after-free in timer_is_static_object+0x80/0x90
kernel/time/timer.c:607
Read of size 8 at addr ffff8801bebb5118 by task syz-executor2/25299
CPU: 1 PID: 25299 Comm: syz-executor2 Not tainted 4.17.0-rc5+ #54
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1b9/0x294 lib/dump_stack.c:113
print_address_description+0x6c/0x20b mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
__asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
timer_is_static_object+0x80/0x90 kernel/time/timer.c:607
debug_object_activate+0x2d9/0x670 lib/debugobjects.c:508
debug_timer_activate kernel/time/timer.c:709 [inline]
debug_activate kernel/time/timer.c:764 [inline]
__mod_timer kernel/time/timer.c:1041 [inline]
mod_timer+0x4d3/0x13b0 kernel/time/timer.c:1102
sk_reset_timer+0x22/0x60 net/core/sock.c:2742
ccid2_hc_tx_rto_expire+0x587/0x680 net/dccp/ccids/ccid2.c:147
call_timer_fn+0x230/0x940 kernel/time/timer.c:1326
expire_timers kernel/time/timer.c:1363 [inline]
__run_timers+0x79e/0xc50 kernel/time/timer.c:1666
run_timer_softirq+0x4c/0x70 kernel/time/timer.c:1692
__do_softirq+0x2e0/0xaf5 kernel/softirq.c:285
invoke_softirq kernel/softirq.c:365 [inline]
irq_exit+0x1d1/0x200 kernel/softirq.c:405
exiting_irq arch/x86/include/asm/apic.h:525 [inline]
smp_apic_timer_interrupt+0x17e/0x710 arch/x86/kernel/apic/apic.c:1052
apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:863
</IRQ>
RIP: 0010:cap_capable+0x3f/0x260 security/commoncap.c:82
RSP: 0018:ffff8801ac75f8c8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
RAX: dffffc0000000000 RBX: ffffffff88d5be00 RCX: 0000000000000000
RDX: 1ffff100386dc571 RSI: ffffffff830ee893 RDI: ffff8801c36e2b88
RBP: ffff8801ac75f910 R08: ffff8801ac126440 R09: ffff8801ac75fcb8
R10: ffff8801ac126c78 R11: ffff8801ac75fc78 R12: dffffc0000000000
R13: dffffc0000000000 R14: ffff8801c36e2b00 R15: 0000000000000021
cap_vm_enough_memory+0x50/0x70 security/commoncap.c:1307
security_vm_enough_memory_mm+0x71/0xc0 security/security.c:327
mmap_region+0x37b/0x1870 mm/mmap.c:1714
do_mmap+0xde2/0x1360 mm/mmap.c:1535
do_mmap_pgoff include/linux/mm.h:2237 [inline]
vm_mmap_pgoff+0x1fb/0x2a0 mm/util.c:357
ksys_mmap_pgoff+0x26e/0x640 mm/mmap.c:1585
__do_sys_mmap arch/x86/kernel/sys_x86_64.c:100 [inline]
__se_sys_mmap arch/x86/kernel/sys_x86_64.c:91 [inline]
__x64_sys_mmap+0xe9/0x1b0 arch/x86/kernel/sys_x86_64.c:91
do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x455a5a
RSP: 002b:0000000000a3e778 EFLAGS: 00000246 ORIG_RAX: 0000000000000009
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000455a5a
RDX: 0000000000000003 RSI: 0000000000021000 RDI: 0000000000000000
RBP: ffffffffffffffff R08: ffffffffffffffff R09: 0000000000000000
R10: 0000000000020022 R11: 0000000000000246 R12: 0000000000000000
R13: 0000000000021000 R14: 0000000000020022 R15: 0000000000000000
Allocated by task 25374:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:490
kmem_cache_alloc+0x12e/0x760 mm/slab.c:3554
ccid_new+0x25b/0x3e0 net/dccp/ccid.c:151
dccp_hdlr_ccid+0x27/0x150 net/dccp/feat.c:44
__dccp_feat_activate+0x184/0x270 net/dccp/feat.c:344
dccp_feat_activate_values+0x3a7/0x819 net/dccp/feat.c:1538
dccp_create_openreq_child+0x472/0x610 net/dccp/minisocks.c:128
dccp_v4_request_recv_sock+0x12c/0xca0 net/dccp/ipv4.c:408
dccp_v6_request_recv_sock+0x125d/0x1f10 net/dccp/ipv6.c:415
dccp_check_req+0x455/0x6a0 net/dccp/minisocks.c:197
dccp_v4_rcv+0x7b8/0x1f3f net/dccp/ipv4.c:841
ip_local_deliver_finish+0x2e3/0xd80 net/ipv4/ip_input.c:215
NF_HOOK include/linux/netfilter.h:288 [inline]
ip_local_deliver+0x1e1/0x720 net/ipv4/ip_input.c:256
dst_input include/net/dst.h:450 [inline]
ip_rcv_finish+0x81b/0x2200 net/ipv4/ip_input.c:396
NF_HOOK include/linux/netfilter.h:288 [inline]
ip_rcv+0xb70/0x143d net/ipv4/ip_input.c:492
__netif_receive_skb_core+0x26f5/0x3630 net/core/dev.c:4592
__netif_receive_skb+0x2c/0x1e0 net/core/dev.c:4657
process_backlog+0x219/0x760 net/core/dev.c:5337
napi_poll net/core/dev.c:5735 [inline]
net_rx_action+0x7b7/0x1930 net/core/dev.c:5801
__do_softirq+0x2e0/0xaf5 kernel/softirq.c:285
Freed by task 25374:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
__kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:521
kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
__cache_free mm/slab.c:3498 [inline]
kmem_cache_free+0x86/0x2d0 mm/slab.c:3756
ccid_hc_tx_delete+0xc3/0x100 net/dccp/ccid.c:190
dccp_disconnect+0x130/0xc66 net/dccp/proto.c:286
dccp_close+0x3bc/0xe60 net/dccp/proto.c:1045
inet_release+0x104/0x1f0 net/ipv4/af_inet.c:427
inet6_release+0x50/0x70 net/ipv6/af_inet6.c:460
sock_release+0x96/0x1b0 net/socket.c:594
sock_close+0x16/0x20 net/socket.c:1149
__fput+0x34d/0x890 fs/file_table.c:209
____fput+0x15/0x20 fs/file_table.c:243
task_work_run+0x1e4/0x290 kernel/task_work.c:113
tracehook_notify_resume include/linux/tracehook.h:191 [inline]
exit_to_usermode_loop+0x2bd/0x310 arch/x86/entry/common.c:166
prepare_exit_to_usermode arch/x86/entry/common.c:196 [inline]
syscall_return_slowpath arch/x86/entry/common.c:265 [inline]
do_syscall_64+0x6ac/0x800 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
The buggy address belongs to the object at ffff8801bebb4cc0
which belongs to the cache ccid2_hc_tx_sock of size 1240
The buggy address is located 1112 bytes inside of
1240-byte region [ffff8801bebb4cc0, ffff8801bebb5198)
The buggy address belongs to the page:
page:ffffea0006faed00 count:1 mapcount:0 mapping:ffff8801bebb41c0
index:0xffff8801bebb5240 compound_mapcount: 0
flags: 0x2fffc0000008100(slab|head)
raw: 02fffc0000008100 ffff8801bebb41c0 ffff8801bebb5240 0000000100000003
raw: ffff8801cdba3138 ffffea0007634120 ffff8801cdbaab40 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8801bebb5000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801bebb5080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8801bebb5100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8801bebb5180: fb fb fb fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff8801bebb5200: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
^ permalink raw reply
* Re: Request for -stable inclusion: time stamping fix for nfp
From: David Miller @ 2018-05-17 18:09 UTC (permalink / raw)
To: g.nault; +Cc: jakub.kicinski, netdev
In-Reply-To: <20180517174147.GC1534@alphalink.fr>
From: Guillaume Nault <g.nault@alphalink.fr>
Date: Thu, 17 May 2018 19:41:47 +0200
> On Thu, Nov 16, 2017 at 10:13:28AM +0900, David Miller wrote:
>> From: Guillaume Nault <g.nault@alphalink.fr>
>> Date: Wed, 15 Nov 2017 17:20:46 +0100
>>
>> > Can you please queue commit 46f1c52e66db
>> > ("nfp: TX time stamp packets before HW doorbell is rung") for -stable?
>> > We got hit but this bug in the late summer. We run this fix internally
>> > since a couple of months, but that'd be better to have it officially
>> > backported so everyone can benefit of it.
>>
>> Queued up.
>
> I guess this one got lost somewhere as it doesn't appear in linux-4.9.y
> (other trees aren't relevant).
> If that's unintentional, than can you please re-queue
> 46f1c52e66db ("nfp: TX time stamp packets before HW doorbell is rung")
> to -stable?
I only submit patches to -stable for the two most recent active branches
which right now consists of 4.16 and 4.14 as per www.kernel.org
^ permalink raw reply
* Re: [bpf-next PATCH 1/2] bpf: allow sk_msg programs to read sock fields
From: Martin KaFai Lau @ 2018-05-17 18:17 UTC (permalink / raw)
To: John Fastabend; +Cc: ast, daniel, netdev
In-Reply-To: <20180517155404.21250.87046.stgit@john-Precision-Tower-5810>
On Thu, May 17, 2018 at 08:54:04AM -0700, John Fastabend wrote:
> Currently sk_msg programs only have access to the raw data. However,
> it is often useful when building policies to have the policies specific
> to the socket endpoint. This allows using the socket tuple as input
> into filters, etc.
>
> This patch adds ctx access to the sock fields.
>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> include/linux/filter.h | 1
> include/uapi/linux/bpf.h | 8 +++
> kernel/bpf/sockmap.c | 1
> net/core/filter.c | 114 +++++++++++++++++++++++++++++++++++++++++++++-
It is indeed a lot of dup lines with sock_ops_convert_ctx_access()
as you mentioned in the cover.
Other than that, LGTM.
Acked-by: Martin KaFai Lau <kafafi@fb.com>
> 4 files changed, 121 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 9dbcb9d..d358d18 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -517,6 +517,7 @@ struct sk_msg_buff {
> bool sg_copy[MAX_SKB_FRAGS];
> __u32 flags;
> struct sock *sk_redir;
> + struct sock *sk;
> struct sk_buff *skb;
> struct list_head list;
> };
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index d94d333..97446bb 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2176,6 +2176,14 @@ enum sk_action {
> struct sk_msg_md {
> void *data;
> void *data_end;
> +
> + __u32 family;
> + __u32 remote_ip4; /* Stored in network byte order */
> + __u32 local_ip4; /* Stored in network byte order */
> + __u32 remote_ip6[4]; /* Stored in network byte order */
> + __u32 local_ip6[4]; /* Stored in network byte order */
> + __u32 remote_port; /* Stored in network byte order */
> + __u32 local_port; /* stored in host byte order */
This ordering inconsistency could be a trap to write bpf_prog
but I guess it is too late to change now considering
bpf_sock_ops is also using this convention.
Just curious, we cannot always assume inet_sk and then uses
its inet_sport?
^ permalink raw reply
* Re: Request for -stable inclusion: time stamping fix for nfp
From: Willy Tarreau @ 2018-05-17 18:32 UTC (permalink / raw)
To: Greg KH; +Cc: David Miller, g.nault, jakub.kicinski, netdev
In-Reply-To: <20180517.140903.1581758299927078395.davem@davemloft.net>
Adding Greg here.
Greg, apparently a backport of 46f1c52e66db is needed in 4.9 according
to the thread below. It was merged in 4.13 so 4.14 already has it.
Willy
On Thu, May 17, 2018 at 02:09:03PM -0400, David Miller wrote:
> From: Guillaume Nault <g.nault@alphalink.fr>
> Date: Thu, 17 May 2018 19:41:47 +0200
>
> > On Thu, Nov 16, 2017 at 10:13:28AM +0900, David Miller wrote:
> >> From: Guillaume Nault <g.nault@alphalink.fr>
> >> Date: Wed, 15 Nov 2017 17:20:46 +0100
> >>
> >> > Can you please queue commit 46f1c52e66db
> >> > ("nfp: TX time stamp packets before HW doorbell is rung") for -stable?
> >> > We got hit but this bug in the late summer. We run this fix internally
> >> > since a couple of months, but that'd be better to have it officially
> >> > backported so everyone can benefit of it.
> >>
> >> Queued up.
> >
> > I guess this one got lost somewhere as it doesn't appear in linux-4.9.y
> > (other trees aren't relevant).
> > If that's unintentional, than can you please re-queue
> > 46f1c52e66db ("nfp: TX time stamp packets before HW doorbell is rung")
> > to -stable?
>
> I only submit patches to -stable for the two most recent active branches
> which right now consists of 4.16 and 4.14 as per www.kernel.org
^ permalink raw reply
* Re: [PATCH 1/2] bpf: sockmap, fix uninitialized variable
From: Gustavo A. R. Silva @ 2018-05-17 18:12 UTC (permalink / raw)
To: John Fastabend, Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, linux-kernel
In-Reply-To: <dd511eb3-abbf-1822-8bcc-7bcc6ca68b6c@gmail.com>
Hi John,
On 05/17/2018 12:27 PM, John Fastabend wrote:
> On 05/17/2018 07:08 AM, Gustavo A. R. Silva wrote:
>> There is a potential execution path in which variable err is
>> returned without being properly initialized previously.
>>
>> Fix this by initializing variable err to 0.
>>
>> Addresses-Coverity-ID: 1468964 ("Uninitialized scalar variable")
>> Fixes: e5cd3abcb31a ("bpf: sockmap, refactor sockmap routines to work
>> with hashmap")
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>> kernel/bpf/sockmap.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
>> index c6de139..41b41fc 100644
>> --- a/kernel/bpf/sockmap.c
>> +++ b/kernel/bpf/sockmap.c
>> @@ -1713,7 +1713,7 @@ static int __sock_map_ctx_update_elem(struct bpf_map *map,
>> struct smap_psock_map_entry *e = NULL;
>> struct smap_psock *psock;
>> bool new = false;
>> - int err;
>> + int err = 0;
>>
>> /* 1. If sock map has BPF programs those will be inherited by the
>> * sock being added. If the sock is already attached to BPF programs
>>
>
> Thanks for catching this and the quick fix. The path to hit this case
> is to add a sock to a map (without a BPF program) where the sock already
> has been added to another map. I don't have any tests for the case with
> socks in multiple maps so I'll add some to the selftests so I remember
> this case.
>
Glad to help. :)
> The alternative fix would be to always 'return 0' at the end of the
> function, but I think its probably better to init err here like above.
>
Yeah. I think initializing err is better in this case.
> Acked-by: John Fastabend <john.fastabend@gmail.com>
>
Thank you
^ permalink raw reply
* Re: [PATCH] wcn36xx: Add support for Factory Test Mode (FTM)
From: Jeff Johnson @ 2018-05-17 18:35 UTC (permalink / raw)
To: Ramon Fried
Cc: kvalo, linux-kernel, wcn36xx, linux-wireless, netdev, Eyal Ilsar,
linux-wireless-owner
In-Reply-To: <20180517113250.16517-1-ramon.fried@linaro.org>
On 2018-05-17 04:32, Ramon Fried wrote:
> From: Eyal Ilsar <eilsar@codeaurora.org>
...
> +static int wcn36xx_smd_process_ptt_msg_rsp(void *buf, size_t len,
> + void **p_ptt_rsp_msg)
> +{
> + struct wcn36xx_hal_process_ptt_msg_rsp_msg *rsp;
> + int ret = 0;
why initialize 'ret' when you immediately overwrite?
> + ret = wcn36xx_smd_rsp_status_check(buf, len);
...
> + if (rsp->header.len > 0) {
> + *p_ptt_rsp_msg = kmalloc(rsp->header.len, GFP_ATOMIC);
NULL check required?
> + memcpy(*p_ptt_rsp_msg, rsp->ptt_msg, rsp->header.len);
> + }
> + return ret;
> +}
> +
> +int wcn36xx_smd_process_ptt_msg(struct wcn36xx *wcn,
> + struct ieee80211_vif *vif, void *ptt_msg, size_t len,
> + void **ptt_rsp_msg)
> +{
> + struct wcn36xx_hal_process_ptt_msg_req_msg *p_msg_body;
> + int ret = 0;
why initialize 'ret' when it is always overwritten before use?
> + ret = wcn36xx_smd_send_and_wait(wcn, p_msg_body->header.len);
^ permalink raw reply
* Re: [PATCH] wcn36xx: Add support for Factory Test Mode (FTM)
From: Jeff Johnson @ 2018-05-17 18:37 UTC (permalink / raw)
To: Ramon Fried
Cc: kvalo, linux-kernel, wcn36xx, linux-wireless, netdev, Eyal Ilsar,
linux-wireless-owner
In-Reply-To: <20180517113250.16517-1-ramon.fried@linaro.org>
On 2018-05-17 04:32, Ramon Fried wrote:
> From: Eyal Ilsar <eilsar@codeaurora.org>
...
> +int wcn36xx_smd_process_ptt_msg(struct wcn36xx *wcn,
> + struct ieee80211_vif *vif, void *ptt_msg, size_t len,
> + void **ptt_rsp_msg)
> +{
> + struct wcn36xx_hal_process_ptt_msg_req_msg *p_msg_body;
> + int ret = 0;
> +
> + mutex_lock(&wcn->hal_mutex);
> + p_msg_body = kmalloc(
> + sizeof(struct wcn36xx_hal_process_ptt_msg_req_msg) + len,
> + GFP_ATOMIC);
NULL check required?
> + INIT_HAL_PTT_MSG(p_msg_body, len);
> +
^ permalink raw reply
* Re: [net-next PATCH v2 1/4] net: Refactor XPS for CPUs and Rx queues
From: David Miller @ 2018-05-17 18:38 UTC (permalink / raw)
To: amritha.nambiar
Cc: netdev, alexander.h.duyck, sridhar.samudrala, edumazet, hannes,
tom
In-Reply-To: <152643400370.4991.2044471541271189575.stgit@anamdev.jf.intel.com>
From: Amritha Nambiar <amritha.nambiar@intel.com>
Date: Tue, 15 May 2018 18:26:43 -0700
> @@ -2125,7 +2125,7 @@ static bool remove_xps_queue_cpu(struct net_device *dev,
> int i, j;
>
> for (i = count, j = offset; i--; j++) {
> - if (!remove_xps_queue(dev_maps, cpu, j))
> + if (!remove_xps_queue(dev_maps, tci, j))
> break;
> }
>
This looks like a bug fix, completely unrelated to the feature being added
by this patch set.
Please submit this targetting the 'net' tree, then when that fix propagates
into 'net-next' you can rebase this series on top of that.
Thank you.
^ permalink raw reply
* Re: [PATCH V2] mlx4_core: allocate ICM memory in page size chunks
From: kbuild test robot @ 2018-05-17 18:39 UTC (permalink / raw)
To: Qing Huang
Cc: kbuild-all, tariqt, davem, haakon.bugge, yanjun.zhu, netdev,
linux-rdma, linux-kernel, Qing Huang
In-Reply-To: <20180511192318.22342-1-qing.huang@oracle.com>
[-- Attachment #1: Type: text/plain, Size: 4003 bytes --]
Hi Qing,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
[also build test ERROR on v4.17-rc5 next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Qing-Huang/mlx4_core-allocate-ICM-memory-in-page-size-chunks/20180512-090438
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64
All error/warnings (new ones prefixed by >>):
drivers/net//ethernet/mellanox/mlx4/icm.c: In function 'mlx4_init_icm_table':
>> drivers/net//ethernet/mellanox/mlx4/icm.c:403:20: error: implicit declaration of function 'vzalloc'; did you mean 'kzalloc'? [-Werror=implicit-function-declaration]
table->icm = vzalloc(num_icm * sizeof(*table->icm));
^~~~~~~
kzalloc
>> drivers/net//ethernet/mellanox/mlx4/icm.c:403:18: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
table->icm = vzalloc(num_icm * sizeof(*table->icm));
^
>> drivers/net//ethernet/mellanox/mlx4/icm.c:449:2: error: implicit declaration of function 'vfree'; did you mean 'kfree'? [-Werror=implicit-function-declaration]
vfree(table->icm);
^~~~~
kfree
cc1: some warnings being treated as errors
vim +403 drivers/net//ethernet/mellanox/mlx4/icm.c
389
390 int mlx4_init_icm_table(struct mlx4_dev *dev, struct mlx4_icm_table *table,
391 u64 virt, int obj_size, u32 nobj, int reserved,
392 int use_lowmem, int use_coherent)
393 {
394 int obj_per_chunk;
395 int num_icm;
396 unsigned chunk_size;
397 int i;
398 u64 size;
399
400 obj_per_chunk = MLX4_TABLE_CHUNK_SIZE / obj_size;
401 num_icm = (nobj + obj_per_chunk - 1) / obj_per_chunk;
402
> 403 table->icm = vzalloc(num_icm * sizeof(*table->icm));
404 if (!table->icm)
405 return -ENOMEM;
406 table->virt = virt;
407 table->num_icm = num_icm;
408 table->num_obj = nobj;
409 table->obj_size = obj_size;
410 table->lowmem = use_lowmem;
411 table->coherent = use_coherent;
412 mutex_init(&table->mutex);
413
414 size = (u64) nobj * obj_size;
415 for (i = 0; i * MLX4_TABLE_CHUNK_SIZE < reserved * obj_size; ++i) {
416 chunk_size = MLX4_TABLE_CHUNK_SIZE;
417 if ((i + 1) * MLX4_TABLE_CHUNK_SIZE > size)
418 chunk_size = PAGE_ALIGN(size -
419 i * MLX4_TABLE_CHUNK_SIZE);
420
421 table->icm[i] = mlx4_alloc_icm(dev, chunk_size >> PAGE_SHIFT,
422 (use_lowmem ? GFP_KERNEL : GFP_HIGHUSER) |
423 __GFP_NOWARN, use_coherent);
424 if (!table->icm[i])
425 goto err;
426 if (mlx4_MAP_ICM(dev, table->icm[i], virt + i * MLX4_TABLE_CHUNK_SIZE)) {
427 mlx4_free_icm(dev, table->icm[i], use_coherent);
428 table->icm[i] = NULL;
429 goto err;
430 }
431
432 /*
433 * Add a reference to this ICM chunk so that it never
434 * gets freed (since it contains reserved firmware objects).
435 */
436 ++table->icm[i]->refcount;
437 }
438
439 return 0;
440
441 err:
442 for (i = 0; i < num_icm; ++i)
443 if (table->icm[i]) {
444 mlx4_UNMAP_ICM(dev, virt + i * MLX4_TABLE_CHUNK_SIZE,
445 MLX4_TABLE_CHUNK_SIZE / MLX4_ICM_PAGE_SIZE);
446 mlx4_free_icm(dev, table->icm[i], use_coherent);
447 }
448
> 449 vfree(table->icm);
450
451 return -ENOMEM;
452 }
453
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 53307 bytes --]
^ permalink raw reply
* Re: [PATCH v2 net-next 00/12] net: stmmac: Clean-up and tune-up
From: David Miller @ 2018-05-17 18:41 UTC (permalink / raw)
To: Jose.Abreu
Cc: f.fainelli, netdev, Joao.Pinto, Vitor.Soares, peppe.cavallaro,
alexandre.torgue
In-Reply-To: <a36d9afa-6939-2695-5f31-bc8777b5f27c@synopsys.com>
From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Thu, 17 May 2018 14:24:42 +0100
> Given that the difference between better/worst is < 1%, I think
> we can conclude patches 3-13 don't affect the overall
> performance. I didn't profile the cache hits/miss though ...
Ok, thanks for making an effort to look into this more thoroughly.
I'll apply this series to net-next, thank you.
^ permalink raw reply
* Re: [PATCH v2 net-next 00/12] net: stmmac: Clean-up and tune-up
From: David Miller @ 2018-05-17 18:47 UTC (permalink / raw)
To: Jose.Abreu
Cc: f.fainelli, netdev, Joao.Pinto, Vitor.Soares, peppe.cavallaro,
alexandre.torgue
In-Reply-To: <20180517.144117.1704300189822624802.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Thu, 17 May 2018 14:41:17 -0400 (EDT)
> From: Jose Abreu <Jose.Abreu@synopsys.com>
> Date: Thu, 17 May 2018 14:24:42 +0100
>
>> Given that the difference between better/worst is < 1%, I think
>> we can conclude patches 3-13 don't affect the overall
>> performance. I didn't profile the cache hits/miss though ...
>
> Ok, thanks for making an effort to look into this more thoroughly.
>
> I'll apply this series to net-next, thank you.
Sorry, I had to revert.
It is one thing to say that you lack the hardware to physically test
the changes on all chip types.
It is yet another to not even test the build properly:
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c:494:10: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
.init = sun8i_dwmac_dma_init,
^~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c:494:10: note: (near initialization for ‘sun8i_dwmac_dma_ops.init’)
cc1: some warnings being treated as errors
^ permalink raw reply
* Re: [net-next PATCH v2 1/4] net: Refactor XPS for CPUs and Rx queues
From: Nambiar, Amritha @ 2018-05-17 18:47 UTC (permalink / raw)
To: David Miller
Cc: netdev, alexander.h.duyck, sridhar.samudrala, edumazet, hannes,
tom
In-Reply-To: <20180517.143853.84077547534626009.davem@davemloft.net>
On 5/17/2018 11:38 AM, David Miller wrote:
> From: Amritha Nambiar <amritha.nambiar@intel.com>
> Date: Tue, 15 May 2018 18:26:43 -0700
>
>> @@ -2125,7 +2125,7 @@ static bool remove_xps_queue_cpu(struct net_device *dev,
>> int i, j;
>>
>> for (i = count, j = offset; i--; j++) {
>> - if (!remove_xps_queue(dev_maps, cpu, j))
>> + if (!remove_xps_queue(dev_maps, tci, j))
>> break;
>> }
>>
>
> This looks like a bug fix, completely unrelated to the feature being added
> by this patch set.
>
> Please submit this targetting the 'net' tree, then when that fix propagates
> into 'net-next' you can rebase this series on top of that.
>
> Thank you.
>
Sure, will do that.
Thanks.
^ permalink raw reply
* Re: [PATCH net] tls: don't use stack memory in a scatterlist
From: David Miller @ 2018-05-17 18:50 UTC (permalink / raw)
To: mmullins; +Cc: ilyal, aviadye, davejwatson, netdev, linux-kernel
In-Reply-To: <20180516174841.2119-1-mmullins@fb.com>
From: Matt Mullins <mmullins@fb.com>
Date: Wed, 16 May 2018 10:48:40 -0700
> scatterlist code expects virt_to_page() to work, which fails with
> CONFIG_VMAP_STACK=y.
>
> Fixes: c46234ebb4d1e ("tls: RX path for ktls")
> Signed-off-by: Matt Mullins <mmullins@fb.com>
Applied and queued up for -stable, thanks.
I'm surprised this problem wasn't discovered sooner. How exactly did you
discover it? Did you actually see it trigger or is this purely from code
inspection?
^ permalink raw reply
* Re: [PATCH bpf 5/6] tools: bpftool: resolve calls without using imm field
From: Jakub Kicinski @ 2018-05-17 18:51 UTC (permalink / raw)
To: Sandipan Das; +Cc: ast, daniel, netdev, linuxppc-dev, naveen.n.rao
In-Reply-To: <20180517063548.6373-6-sandipan@linux.vnet.ibm.com>
On Thu, 17 May 2018 12:05:47 +0530, Sandipan Das wrote:
> Currently, we resolve the callee's address for a JITed function
> call by using the imm field of the call instruction as an offset
> from __bpf_call_base. If bpf_jit_kallsyms is enabled, we further
> use this address to get the callee's kernel symbol's name.
>
> For some architectures, such as powerpc64, the imm field is not
> large enough to hold this offset. So, instead of assigning this
> offset to the imm field, the verifier now assigns the subprog
> id. Also, a list of kernel symbol addresses for all the JITed
> functions is provided in the program info. We now use the imm
> field as an index for this list to lookup a callee's symbol's
> address and resolve its name.
>
> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com>
A few nit-picks below, thank you for the patch!
> tools/bpf/bpftool/prog.c | 31 +++++++++++++++++++++++++++++++
> tools/bpf/bpftool/xlated_dumper.c | 24 +++++++++++++++++-------
> tools/bpf/bpftool/xlated_dumper.h | 2 ++
> 3 files changed, 50 insertions(+), 7 deletions(-)
>
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 9bdfdf2d3fbe..ac2f62a97e84 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -430,6 +430,10 @@ static int do_dump(int argc, char **argv)
> unsigned char *buf;
> __u32 *member_len;
> __u64 *member_ptr;
> + unsigned int nr_addrs;
> + unsigned long *addrs = NULL;
> + __u32 *ksyms_len;
> + __u64 *ksyms_ptr;
nit: please try to keep the variables ordered longest to shortest like
we do in networking code (please do it in all functions).
> ssize_t n;
> int err;
> int fd;
> @@ -437,6 +441,8 @@ static int do_dump(int argc, char **argv)
> if (is_prefix(*argv, "jited")) {
> member_len = &info.jited_prog_len;
> member_ptr = &info.jited_prog_insns;
> + ksyms_len = &info.nr_jited_ksyms;
> + ksyms_ptr = &info.jited_ksyms;
> } else if (is_prefix(*argv, "xlated")) {
> member_len = &info.xlated_prog_len;
> member_ptr = &info.xlated_prog_insns;
> @@ -496,10 +502,23 @@ static int do_dump(int argc, char **argv)
> return -1;
> }
>
> + nr_addrs = *ksyms_len;
Here and ...
> + if (nr_addrs) {
> + addrs = malloc(nr_addrs * sizeof(__u64));
> + if (!addrs) {
> + p_err("mem alloc failed");
> + free(buf);
> + close(fd);
> + return -1;
You can just jump to err_free here.
> + }
> + }
> +
> memset(&info, 0, sizeof(info));
>
> *member_ptr = ptr_to_u64(buf);
> *member_len = buf_size;
> + *ksyms_ptr = ptr_to_u64(addrs);
> + *ksyms_len = nr_addrs;
... here - this function is getting long, so maybe I'm not seeing
something, but are ksyms_ptr and ksyms_len guaranteed to be initialized?
> err = bpf_obj_get_info_by_fd(fd, &info, &len);
> close(fd);
> @@ -513,6 +532,11 @@ static int do_dump(int argc, char **argv)
> goto err_free;
> }
>
> + if (*ksyms_len > nr_addrs) {
> + p_err("too many addresses returned");
> + goto err_free;
> + }
> +
> if ((member_len == &info.jited_prog_len &&
> info.jited_prog_insns == 0) ||
> (member_len == &info.xlated_prog_len &&
> @@ -558,6 +582,9 @@ static int do_dump(int argc, char **argv)
> dump_xlated_cfg(buf, *member_len);
> } else {
> kernel_syms_load(&dd);
> + dd.jited_ksyms = ksyms_ptr;
> + dd.nr_jited_ksyms = *ksyms_len;
> +
> if (json_output)
> dump_xlated_json(&dd, buf, *member_len, opcodes);
> else
> @@ -566,10 +593,14 @@ static int do_dump(int argc, char **argv)
> }
>
> free(buf);
> + if (addrs)
> + free(addrs);
Free can deal with NULL pointers, no need for an if.
> return 0;
>
> err_free:
> free(buf);
> + if (addrs)
> + free(addrs);
> return -1;
> }
>
> diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c
> index 7a3173b76c16..dc8e4eca0387 100644
> --- a/tools/bpf/bpftool/xlated_dumper.c
> +++ b/tools/bpf/bpftool/xlated_dumper.c
> @@ -178,8 +178,12 @@ static const char *print_call_pcrel(struct dump_data *dd,
> snprintf(dd->scratch_buff, sizeof(dd->scratch_buff),
> "%+d#%s", insn->off, sym->name);
> else
else if (address)
saves us the indentation.
> - snprintf(dd->scratch_buff, sizeof(dd->scratch_buff),
> - "%+d#0x%lx", insn->off, address);
> + if (address)
> + snprintf(dd->scratch_buff, sizeof(dd->scratch_buff),
> + "%+d#0x%lx", insn->off, address);
> + else
> + snprintf(dd->scratch_buff, sizeof(dd->scratch_buff),
> + "%+d", insn->off);
> return dd->scratch_buff;
> }
>
> @@ -200,14 +204,20 @@ static const char *print_call(void *private_data,
> const struct bpf_insn *insn)
> {
> struct dump_data *dd = private_data;
> - unsigned long address = dd->address_call_base + insn->imm;
> - struct kernel_sym *sym;
> + unsigned long address = 0;
> + struct kernel_sym *sym = NULL;
>
Hm. Quite a bit of churn. Why not just add these three lines here:
if (insn->src_reg == BPF_PSEUDO_CALL &&
insn->imm < dd->nr_jited_ksyms)
address = dd->jited_ksyms[insn->imm];
> - sym = kernel_syms_search(dd, address);
> - if (insn->src_reg == BPF_PSEUDO_CALL)
> + if (insn->src_reg == BPF_PSEUDO_CALL) {
> + if (dd->nr_jited_ksyms) {
> + address = dd->jited_ksyms[insn->imm];
Perhaps it's paranoid, but it'd please do to bound check insn->imm
against dd->nr_jited_ksyms.
> + sym = kernel_syms_search(dd, address);
> + }
> return print_call_pcrel(dd, sym, address, insn);
> - else
> + } else {
> + address = dd->address_call_base + insn->imm;
> + sym = kernel_syms_search(dd, address);
> return print_call_helper(dd, sym, address);
> + }
> }
>
> static const char *print_imm(void *private_data,
^ 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