* Re: [PATCH 3/8] socket: Disentangle SOCK_RCVTSTAMPNS from SOCK_RCVTSTAMP
From: Willem de Bruijn @ 2018-11-25 3:59 UTC (permalink / raw)
To: Deepa Dinamani
Cc: David Miller, LKML, Network Development, Al Viro, Arnd Bergmann,
y2038 Mailman List
In-Reply-To: <20181124022035.17519-4-deepa.kernel@gmail.com>
On Sat, Nov 24, 2018 at 3:59 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
>
> SOCK_RCVTSTAMPNS is never set alone. SOCK_RCVTSTAMP
> is always set along with SOCK_RCVTSTAMPNS. This leads to
> checking for two flag states whenever we need to check for
> SOCK_RCVTSTAMPS.
>
> Also SOCK_RCVTSTAMPS was the only flag that needed to be
> checked in order to verify if either of the two flags are
> set. But, the two features are not actually dependent on
> each other. This artificial dependency creates more
> confusion.
This is done so that the hot path only has to check one flag
in the common case where no timestamp is requested.
^ permalink raw reply
* Re: [PATCH 7/8] socket: Add SO_TIMESTAMP[NS]_NEW
From: Willem de Bruijn @ 2018-11-25 3:59 UTC (permalink / raw)
To: Deepa Dinamani
Cc: David Miller, LKML, Network Development, Al Viro, Arnd Bergmann,
y2038 Mailman List, jejb, ralf, rth, linux-alpha, linux-mips,
linux-parisc, linux-rdma, sparclinux
In-Reply-To: <20181124022035.17519-8-deepa.kernel@gmail.com>
On Sat, Nov 24, 2018 at 3:58 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
>
> Add SO_TIMESTAMP_NEW and SO_TIMESTAMPNS_NEW variants of
> socket timestamp options.
> These are the y2038 safe versions of the SO_TIMESTAMP_OLD
> and SO_TIMESTAMPNS_OLD for all architectures.
>
> Note that the format of scm_timestamping.ts[0] is not changed
> in this patch.
>
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> Cc: jejb@parisc-linux.org
> Cc: ralf@linux-mips.org
> Cc: rth@twiddle.net
> Cc: linux-alpha@vger.kernel.org
> Cc: linux-mips@linux-mips.org
> Cc: linux-parisc@vger.kernel.org
> Cc: linux-rdma@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Cc: sparclinux@vger.kernel.org
> ---
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 8143c4c1a49d..9edf909dc176 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -801,6 +801,7 @@ enum sock_flags {
> SOCK_RCU_FREE, /* wait rcu grace period in sk_destruct() */
> SOCK_TXTIME,
> SOCK_XDP, /* XDP is attached */
> + SOCK_TSTAMP_NEW, /* Indicates 64 bit timestamps always */
sk_flags is getting exhausted. Commit b9f40e21ef42 ("net-timestamp:
move timestamp flags out of sk_flags") added a new u16 sk_tsflags
specifically for timestamps. That may be a better choice here, too.
> diff --git a/net/core/sock.c b/net/core/sock.c
> index e60036618205..7b485dfaa400 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -652,15 +652,23 @@ static void setsockopt_timestamp(struct sock *sk, int type, int val)
> if (!val) {
> sock_reset_flag(sk, SOCK_RCVTSTAMP);
> sock_reset_flag(sk, SOCK_RCVTSTAMPNS);
> + sock_reset_flag(sk, SOCK_TSTAMP_NEW);
> return;
> }
>
> + if (type == SO_TIMESTAMP_NEW || type == SO_TIMESTAMPNS_NEW)
> + sock_set_flag(sk, SOCK_TSTAMP_NEW);
> + else
> + sock_reset_flag(sk, SOCK_TSTAMP_NEW);
> +
if adding a boolean whether the socket uses new or old-style
timestamps, perhaps fail hard if a process tries to set a new-style
option while an old-style is already set and vice versa. Also include
SO_TIMESTAMPING_NEW as it toggles the same option.
> diff --git a/net/socket.c b/net/socket.c
> index d3defba55547..9abeb6bc9cfe 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -699,6 +699,38 @@ static void put_ts_pktinfo(struct msghdr *msg, struct sk_buff *skb)
> sizeof(ts_pktinfo), &ts_pktinfo);
> }
>
> +static void sock_recv_sw_timestamp(struct msghdr *msg, struct sock *sk,
> + struct sk_buff *skb)
> +{
> + if (sock_flag(sk, SOCK_TSTAMP_NEW)) {
> + if (!sock_flag(sk, SOCK_RCVTSTAMPNS)) {
> + struct sock_timeval tv;
> +
> + skb_get_new_timestamp(skb, &tv);
> + put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMP_NEW,
> + sizeof(tv), &tv);
> + } else {
> + struct __kernel_timespec ts;
> +
> + skb_get_new_timestampns(skb, &ts);
> + put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMPNS_NEW,
> + sizeof(ts), &ts);
> + }
> + }
> + if (!sock_flag(sk, SOCK_RCVTSTAMPNS)) {
> + struct __kernel_old_timeval tv;
> +
> + skb_get_timestamp(skb, &tv);
> + put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMP_OLD,
> + sizeof(tv), &tv);
> + } else {
> + struct timespec ts;
> +
> + skb_get_timestampns(skb, &ts);
> + put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMPNS_OLD,
> + sizeof(ts), &ts);
> + }
> +}
> /*
> * called from sock_recv_timestamp() if sock_flag(sk, SOCK_RCVTSTAMP)
> * or sock_flag(sk, SOCK_RCVTSTAMPNS)
> @@ -719,19 +751,8 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
> false_tstamp = 1;
> }
> - if (need_software_tstamp) {
Considerably less code churn if adding __sock_recv_timestamp_2038 and
calling that here:
if (sock_flag(sk, SOCK_TSTAMP_NEW))
__sock_recv_timestamp_2038(msg, sk, skb);
else if ...
Same for the tcp case above, really, and in the case of the next patch
for SO_TIMESTAMPING_NEW
> - if (!sock_flag(sk, SOCK_RCVTSTAMPNS)) {
> - struct __kernel_old_timeval tv;
> - skb_get_timestamp(skb, &tv);
> - put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMP_OLD,
> - sizeof(tv), &tv);
> - } else {
> - struct timespec ts;
> - skb_get_timestampns(skb, &ts);
> - put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMPNS_OLD,
> - sizeof(ts), &ts);
> - }
> - }
> + if (need_software_tstamp)
> + sock_recv_sw_timestamp(msg, sk, skb);
>
> memset(&tss, 0, sizeof(tss));
> if ((sk->sk_tsflags & SOF_TIMESTAMPING_SOFTWARE) &&
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH 8/8] socket: Add SO_TIMESTAMPING_NEW
From: Willem de Bruijn @ 2018-11-25 4:00 UTC (permalink / raw)
To: Deepa Dinamani
Cc: David Miller, LKML, Network Development, Al Viro, Arnd Bergmann,
y2038 Mailman List, chris, fenghua.yu, rth, Thomas Gleixner,
ubraun, linux-alpha, linux-arch, linux-ia64, linux-mips,
linux-s390, linux-xtensa, sparclinux
In-Reply-To: <20181124022035.17519-9-deepa.kernel@gmail.com>
On Sat, Nov 24, 2018 at 3:58 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
>
> Add SO_TIMESTAMPING_NEW variant of socket timestamp options.
> This is the y2038 safe versions of the SO_TIMESTAMPING_OLD
> for all architectures.
>
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> Cc: chris@zankel.net
> Cc: fenghua.yu@intel.com
> Cc: rth@twiddle.net
> Cc: tglx@linutronix.de
> Cc: ubraun@linux.ibm.com
> Cc: linux-alpha@vger.kernel.org
> Cc: linux-arch@vger.kernel.org
> Cc: linux-ia64@vger.kernel.org
> Cc: linux-mips@linux-mips.org
> Cc: linux-s390@vger.kernel.org
> Cc: linux-xtensa@linux-xtensa.org
> Cc: sparclinux@vger.kernel.org
> ---
> /*
> * called from sock_recv_timestamp() if sock_flag(sk, SOCK_RCVTSTAMP)
> * or sock_flag(sk, SOCK_RCVTSTAMPNS)
> @@ -739,8 +740,8 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
> struct sk_buff *skb)
> {
> int need_software_tstamp = sock_flag(sk, SOCK_RCVTSTAMP) || sock_flag(sk, SOCK_RCVTSTAMPNS);
> - struct scm_timestamping tss;
> - int empty = 1, false_tstamp = 0;
> + struct scm_timestamping_internal tss;
> + int empty = 1, false_tstamp = 0, new_tstamp = 0;
> struct skb_shared_hwtstamps *shhwtstamps =
> skb_hwtstamps(skb);
>
> @@ -756,20 +757,23 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
>
> memset(&tss, 0, sizeof(tss));
> if ((sk->sk_tsflags & SOF_TIMESTAMPING_SOFTWARE) &&
> - ktime_to_timespec_cond(skb->tstamp, tss.ts + 0))
> + ktime_to_timespec64_cond(skb->tstamp, tss.ts + 0))
> empty = 0;
> if (shhwtstamps &&
> (sk->sk_tsflags & SOF_TIMESTAMPING_RAW_HARDWARE) &&
> !skb_is_swtx_tstamp(skb, false_tstamp) &&
> - ktime_to_timespec_cond(shhwtstamps->hwtstamp, tss.ts + 2)) {
> + ktime_to_timespec64_cond(shhwtstamps->hwtstamp, tss.ts + 2)) {
> empty = 0;
> if ((sk->sk_tsflags & SOF_TIMESTAMPING_OPT_PKTINFO) &&
> !skb_is_err_queue(skb))
> put_ts_pktinfo(msg, skb);
> }
> if (!empty) {
> - put_cmsg(msg, SOL_SOCKET,
> - SO_TIMESTAMPING_OLD, sizeof(tss), &tss);
> + new_tstamp = sock_flag(sk, SOCK_TSTAMP_NEW);
> + if (new_tstamp)
nit: no need for explicit variable
> + put_cmsg_scm_timestamping64(msg, &tss);
> + else
> + put_cmsg_scm_timestamping(msg, &tss);
>
> if (skb_is_err_queue(skb) && skb->len &&
> SKB_EXT_ERR(skb)->opt_stats)
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH net] tcp: address problems caused by EDT misshaps
From: Eric Dumazet @ 2018-11-24 17:12 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Neal Cardwell, Yuchung Cheng, Eric Dumazet, Eric Dumazet
When a qdisc setup including pacing FQ is dismantled and recreated,
some TCP packets are sent earlier than instructed by TCP stack.
TCP can be fooled when ACK comes back, because the following
operation can return a negative value.
tcp_time_stamp(tp) - tp->rx_opt.rcv_tsecr;
Some paths in TCP stack were not dealing properly with this,
this patch addresses four of them.
Fixes: ab408b6dc744 ("tcp: switch tcp and sch_fq to new earliest departure time model")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_input.c | 16 ++++++++++------
net/ipv4/tcp_timer.c | 10 ++++++----
2 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 1e37c13881893c8a73c0fcaa82a9979d3b5e1798..a9d9555a973fed4e3562a57d1a2cdadfef40dae4 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -579,10 +579,12 @@ static inline void tcp_rcv_rtt_measure_ts(struct sock *sk,
u32 delta = tcp_time_stamp(tp) - tp->rx_opt.rcv_tsecr;
u32 delta_us;
- if (!delta)
- delta = 1;
- delta_us = delta * (USEC_PER_SEC / TCP_TS_HZ);
- tcp_rcv_rtt_update(tp, delta_us, 0);
+ if (likely(delta < INT_MAX / (USEC_PER_SEC / TCP_TS_HZ))) {
+ if (!delta)
+ delta = 1;
+ delta_us = delta * (USEC_PER_SEC / TCP_TS_HZ);
+ tcp_rcv_rtt_update(tp, delta_us, 0);
+ }
}
}
@@ -2910,9 +2912,11 @@ static bool tcp_ack_update_rtt(struct sock *sk, const int flag,
if (seq_rtt_us < 0 && tp->rx_opt.saw_tstamp && tp->rx_opt.rcv_tsecr &&
flag & FLAG_ACKED) {
u32 delta = tcp_time_stamp(tp) - tp->rx_opt.rcv_tsecr;
- u32 delta_us = delta * (USEC_PER_SEC / TCP_TS_HZ);
- seq_rtt_us = ca_rtt_us = delta_us;
+ if (likely(delta < INT_MAX / (USEC_PER_SEC / TCP_TS_HZ))) {
+ seq_rtt_us = delta * (USEC_PER_SEC / TCP_TS_HZ);
+ ca_rtt_us = seq_rtt_us;
+ }
}
rs->rtt_us = ca_rtt_us; /* RTT of last (S)ACKed packet (or -1) */
if (seq_rtt_us < 0)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 5f8b6d3cd855dc639409e69d84ade5bb2be51626..091c53925e4da6b2b154d166682a0ac0aefd7ecb 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -40,15 +40,17 @@ static u32 tcp_clamp_rto_to_user_timeout(const struct sock *sk)
{
struct inet_connection_sock *icsk = inet_csk(sk);
u32 elapsed, start_ts;
+ s32 remaining;
start_ts = tcp_retransmit_stamp(sk);
if (!icsk->icsk_user_timeout || !start_ts)
return icsk->icsk_rto;
elapsed = tcp_time_stamp(tcp_sk(sk)) - start_ts;
- if (elapsed >= icsk->icsk_user_timeout)
+ remaining = icsk->icsk_user_timeout - elapsed;
+ if (remaining <= 0)
return 1; /* user timeout has passed; fire ASAP */
- else
- return min_t(u32, icsk->icsk_rto, msecs_to_jiffies(icsk->icsk_user_timeout - elapsed));
+
+ return min_t(u32, icsk->icsk_rto, msecs_to_jiffies(remaining));
}
/**
@@ -209,7 +211,7 @@ static bool retransmits_timed_out(struct sock *sk,
(boundary - linear_backoff_thresh) * TCP_RTO_MAX;
timeout = jiffies_to_msecs(timeout);
}
- return (tcp_time_stamp(tcp_sk(sk)) - start_ts) >= timeout;
+ return (s32)(tcp_time_stamp(tcp_sk(sk)) - start_ts - timeout) >= 0;
}
/* A write timeout has occurred. Process the after effects. */
--
2.20.0.rc0.387.gc7a69e6b6c-goog
^ permalink raw reply related
* Re: [PATCH 7/8] socket: Add SO_TIMESTAMP[NS]_NEW
From: Willem de Bruijn @ 2018-11-25 4:17 UTC (permalink / raw)
To: Deepa Dinamani
Cc: linux-mips, linux-parisc, Arnd Bergmann, y2038 Mailman List,
Network Development, LKML, ralf, jejb, linux-rdma, Al Viro,
linux-alpha, sparclinux, David Miller, rth
In-Reply-To: <CAF=yD-K=ZLZ_S=Y6YnhvvkfKQS=21ujwiGd7+JB9yDMtiX_9hg@mail.gmail.com>
On Sat, Nov 24, 2018 at 10:59 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Sat, Nov 24, 2018 at 3:58 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> >
> > Add SO_TIMESTAMP_NEW and SO_TIMESTAMPNS_NEW variants of
> > socket timestamp options.
> > These are the y2038 safe versions of the SO_TIMESTAMP_OLD
> > and SO_TIMESTAMPNS_OLD for all architectures.
> >
> > Note that the format of scm_timestamping.ts[0] is not changed
> > in this patch.
> >
> > Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> > Cc: jejb@parisc-linux.org
> > Cc: ralf@linux-mips.org
> > Cc: rth@twiddle.net
> > Cc: linux-alpha@vger.kernel.org
> > Cc: linux-mips@linux-mips.org
> > Cc: linux-parisc@vger.kernel.org
> > Cc: linux-rdma@vger.kernel.org
> > Cc: netdev@vger.kernel.org
> > Cc: sparclinux@vger.kernel.org
> > ---
>
> > diff --git a/include/net/sock.h b/include/net/sock.h
> > index 8143c4c1a49d..9edf909dc176 100644
> > --- a/include/net/sock.h
> > +++ b/include/net/sock.h
> > @@ -801,6 +801,7 @@ enum sock_flags {
> > SOCK_RCU_FREE, /* wait rcu grace period in sk_destruct() */
> > SOCK_TXTIME,
> > SOCK_XDP, /* XDP is attached */
> > + SOCK_TSTAMP_NEW, /* Indicates 64 bit timestamps always */
>
> sk_flags is getting exhausted. Commit b9f40e21ef42 ("net-timestamp:
> move timestamp flags out of sk_flags") added a new u16 sk_tsflags
> specifically for timestamps. That may be a better choice here, too.
>
> > diff --git a/net/core/sock.c b/net/core/sock.c
> > index e60036618205..7b485dfaa400 100644
> > --- a/net/core/sock.c
> > +++ b/net/core/sock.c
> > @@ -652,15 +652,23 @@ static void setsockopt_timestamp(struct sock *sk, int type, int val)
> > if (!val) {
> > sock_reset_flag(sk, SOCK_RCVTSTAMP);
> > sock_reset_flag(sk, SOCK_RCVTSTAMPNS);
> > + sock_reset_flag(sk, SOCK_TSTAMP_NEW);
> > return;
> > }
> >
> > + if (type == SO_TIMESTAMP_NEW || type == SO_TIMESTAMPNS_NEW)
> > + sock_set_flag(sk, SOCK_TSTAMP_NEW);
> > + else
> > + sock_reset_flag(sk, SOCK_TSTAMP_NEW);
> > +
>
> if adding a boolean whether the socket uses new or old-style
> timestamps, perhaps fail hard if a process tries to set a new-style
> option while an old-style is already set and vice versa. Also include
> SO_TIMESTAMPING_NEW as it toggles the same option.
>
> > diff --git a/net/socket.c b/net/socket.c
> > index d3defba55547..9abeb6bc9cfe 100644
> > --- a/net/socket.c
> > +++ b/net/socket.c
> > @@ -699,6 +699,38 @@ static void put_ts_pktinfo(struct msghdr *msg, struct sk_buff *skb)
> > sizeof(ts_pktinfo), &ts_pktinfo);
> > }
> >
> > +static void sock_recv_sw_timestamp(struct msghdr *msg, struct sock *sk,
> > + struct sk_buff *skb)
> > +{
> > + if (sock_flag(sk, SOCK_TSTAMP_NEW)) {
> > + if (!sock_flag(sk, SOCK_RCVTSTAMPNS)) {
> > + struct sock_timeval tv;
> > +
> > + skb_get_new_timestamp(skb, &tv);
> > + put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMP_NEW,
> > + sizeof(tv), &tv);
> > + } else {
> > + struct __kernel_timespec ts;
> > +
> > + skb_get_new_timestampns(skb, &ts);
> > + put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMPNS_NEW,
> > + sizeof(ts), &ts);
> > + }
> > + }
> > + if (!sock_flag(sk, SOCK_RCVTSTAMPNS)) {
> > + struct __kernel_old_timeval tv;
> > +
> > + skb_get_timestamp(skb, &tv);
> > + put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMP_OLD,
> > + sizeof(tv), &tv);
> > + } else {
> > + struct timespec ts;
> > +
> > + skb_get_timestampns(skb, &ts);
> > + put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMPNS_OLD,
> > + sizeof(ts), &ts);
> > + }
> > +}
> > /*
> > * called from sock_recv_timestamp() if sock_flag(sk, SOCK_RCVTSTAMP)
> > * or sock_flag(sk, SOCK_RCVTSTAMPNS)
> > @@ -719,19 +751,8 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
> > false_tstamp = 1;
> > }
> > - if (need_software_tstamp) {
>
> Considerably less code churn if adding __sock_recv_timestamp_2038 and
> calling that here:
>
> if (sock_flag(sk, SOCK_TSTAMP_NEW))
> __sock_recv_timestamp_2038(msg, sk, skb);
> else if ...
>
> Same for the tcp case above, really, and in the case of the next patch
> for SO_TIMESTAMPING_NEW
That naming convention, ..._2038, is not the nicest, of course. That
is not the relevant bit in the above comment.
Come to think of it, and related to my question in patch 2 why the
need to rename at all, could all new structs, constants and functions
be named consistently with 64 suffix? __sock_recv_timestamp64,
SO_TIMESTAMPING64 and timeval64 (instead of sock_timeval,
it isn't really a sock specific struct)?
I guess that there is a good reason for the renaming exercise and
conditional mapping of SO_TIMESTAMP onto old or new interface.
Please elucidate in the commit message.
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply
* Re: Mrs.Amelia George
From: Mrs.Amelia George @ 2018-11-24 13:14 UTC (permalink / raw)
--
Dear Sir,
My name is Mrs.Amelia George I am 72 years old, I am a dying woman who
have decided to donate what I have to you/churches/ motherless
babies/less
privileged/widows.
I have been touched by God to donate from what I have inherited from my
late husband to you for good work of God.
Please kindly let me know if you can assist me receive my funds and
distribute to charity of your choice the sum of $25million United state
Dollars. I am willing to give you 20% for your time and effort.
Your sister in Christ,
Mrs.Amelia George
Email:dr.amelia.george1@yandex.ru
^ permalink raw reply
* Re: [PATCH 6/8] socket: Add struct sock_timeval
From: Deepa Dinamani @ 2018-11-25 4:52 UTC (permalink / raw)
To: willemdebruijn.kernel
Cc: Arnd Bergmann, y2038 Mailman List,
Linux Network Devel Mailing List, Linux Kernel Mailing List,
Alexander Viro, David S. Miller
In-Reply-To: <CAF=yD-+1BJHuajcW=2bpeAp5XOAX2fObyvhOdH-YsiGY3-_J4Q@mail.gmail.com>
On Sat, Nov 24, 2018 at 11:38 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Sat, Nov 24, 2018 at 4:00 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> >
> > The new type is meant to be used as a y2038 safe structure
> > to be used as part of cmsg data.
> > Presently the SO_TIMESTAMP socket option uses struct timeval
> > for timestamps. This is not y2038 safe.
> > Subsequent patches in the series add new y2038 safe socket
> > option to be used in the place of SO_TIMESTAMP_OLD.
> > struct sock_timeval will be used as the timestamp format
> > at that time.
> >
> > struct sock_timeval also maintains the same layout across
> > 32 bit and 64 bit ABIs.
> >
> > Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> > ---
> > include/uapi/linux/time.h | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/include/uapi/linux/time.h b/include/uapi/linux/time.h
> > index 04d5587f30d3..106f9398c285 100644
> > --- a/include/uapi/linux/time.h
> > +++ b/include/uapi/linux/time.h
> > @@ -70,6 +70,11 @@ struct __kernel_old_timeval {
> > };
> > #endif
> >
> > +struct sock_timeval {
> > + long long tv_sec;
> > + long long tv_usec;
>
> should these use fixed-width type __u64?
We have avoided using __u64/__s64 types for time types in uapi.
I think we did this for portability reasons.
Although this new type might not be required to be interpreted in
libc, I would prefer for this to be long long.
If there is a strong preference then I can change it.
-Deepa
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply
* Re: [PATCH 8/8] socket: Add SO_TIMESTAMPING_NEW
From: Deepa Dinamani @ 2018-11-25 5:07 UTC (permalink / raw)
To: willemdebruijn.kernel
Cc: David S. Miller, Linux Kernel Mailing List,
Linux Network Devel Mailing List, Alexander Viro, Arnd Bergmann,
y2038 Mailman List, Chris Zankel, fenghua.yu, rth,
Thomas Gleixner, ubraun, linux-alpha, linux-arch, linux-ia64,
open list:RALINK MIPS ARCHITECTURE, linux-s390, linux-xtensa,
sparclinux
In-Reply-To: <CAF=yD-Jz8c2EsoKxUDbf7dygWRW49kwteUsksq3eUV00Eg94-w@mail.gmail.com>
> > + new_tstamp = sock_flag(sk, SOCK_TSTAMP_NEW);
> > + if (new_tstamp)
>
> nit: no need for explicit variable
Sure, will drop it.
-Deepa
^ permalink raw reply
* Re: [PATCH 7/8] socket: Add SO_TIMESTAMP[NS]_NEW
From: Deepa Dinamani @ 2018-11-25 5:28 UTC (permalink / raw)
To: willemdebruijn.kernel
Cc: open list:RALINK MIPS ARCHITECTURE, Parisc List, Arnd Bergmann,
y2038 Mailman List, Linux Network Devel Mailing List,
Linux Kernel Mailing List, Ralf Baechle, James E.J. Bottomley,
linux-rdma, Alexander Viro, linux-alpha, sparclinux,
David S. Miller, rth
In-Reply-To: <CAF=yD-K3Ga8qojSh5Epa6m4EYn-Lvdo=e38MnFqOKch1128VHw@mail.gmail.com>
> > > + if (type == SO_TIMESTAMP_NEW || type == SO_TIMESTAMPNS_NEW)
> > > + sock_set_flag(sk, SOCK_TSTAMP_NEW);
> > > + else
> > > + sock_reset_flag(sk, SOCK_TSTAMP_NEW);
> > > +
> >
> > if adding a boolean whether the socket uses new or old-style
> > timestamps, perhaps fail hard if a process tries to set a new-style
> > option while an old-style is already set and vice versa. Also include
> > SO_TIMESTAMPING_NEW as it toggles the same option.
I do not think this is a problem.
Consider this example, if there is a user application with updated
socket timestamps is linking into a library that is yet to be updated.
Besides, the old timestamps should work perfectly fine on 64 bit
arches even beyond 2038.
So failing here means adding a bunch of ifdef's to verify it is not
executing on 64 bit arch or something like x32.
> > > diff --git a/net/socket.c b/net/socket.c
> > > index d3defba55547..9abeb6bc9cfe 100644
> > > --- a/net/socket.c
> > > +++ b/net/socket.c
> > > @@ -699,6 +699,38 @@ static void put_ts_pktinfo(struct msghdr *msg, struct sk_buff *skb)
> > > sizeof(ts_pktinfo), &ts_pktinfo);
> > > }
> > >
> > > +static void sock_recv_sw_timestamp(struct msghdr *msg, struct sock *sk,
> > > + struct sk_buff *skb)
> > > +{
> > > + if (sock_flag(sk, SOCK_TSTAMP_NEW)) {
> > > + if (!sock_flag(sk, SOCK_RCVTSTAMPNS)) {
> > > + struct sock_timeval tv;
> > > +
> > > + skb_get_new_timestamp(skb, &tv);
> > > + put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMP_NEW,
> > > + sizeof(tv), &tv);
> > > + } else {
> > > + struct __kernel_timespec ts;
> > > +
> > > + skb_get_new_timestampns(skb, &ts);
> > > + put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMPNS_NEW,
> > > + sizeof(ts), &ts);
> > > + }
> > > + }
> > > + if (!sock_flag(sk, SOCK_RCVTSTAMPNS)) {
> > > + struct __kernel_old_timeval tv;
> > > +
> > > + skb_get_timestamp(skb, &tv);
> > > + put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMP_OLD,
> > > + sizeof(tv), &tv);
> > > + } else {
> > > + struct timespec ts;
> > > +
> > > + skb_get_timestampns(skb, &ts);
> > > + put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMPNS_OLD,
> > > + sizeof(ts), &ts);
> > > + }
> > > +}
> > > /*
> > > * called from sock_recv_timestamp() if sock_flag(sk, SOCK_RCVTSTAMP)
> > > * or sock_flag(sk, SOCK_RCVTSTAMPNS)
> > > @@ -719,19 +751,8 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
> > > false_tstamp = 1;
> > > }
> > > - if (need_software_tstamp) {
> >
> > Considerably less code churn if adding __sock_recv_timestamp_2038 and
> > calling that here:
> >
> > if (sock_flag(sk, SOCK_TSTAMP_NEW))
> > __sock_recv_timestamp_2038(msg, sk, skb);
> > else if ...
> >
> > Same for the tcp case above, really, and in the case of the next patch
> > for SO_TIMESTAMPING_NEW
>
> That naming convention, ..._2038, is not the nicest, of course. That
> is not the relevant bit in the above comment.
>
> Come to think of it, and related to my question in patch 2 why the
> need to rename at all, could all new structs, constants and functions
> be named consistently with 64 suffix? __sock_recv_timestamp64,
> SO_TIMESTAMPING64 and timeval64 (instead of sock_timeval,
> it isn't really a sock specific struct)?
>
> I guess that there is a good reason for the renaming exercise and
> conditional mapping of SO_TIMESTAMP onto old or new interface.
> Please elucidate in the commit message.
I think there is some confusion here.
The existing timestamp options: SO_TIMESTAMP* fail to provide proper
timestamps beyond year 2038 on 32 bit ABIs.
But, these work fine on 64 bit native ABIs.
So now we need a way of updating these timestamps so that we do not
break existing userspace: 64 bit ABIs should not have to change
userspace, 32 bit ABIs should work as is until 2038 after which they
have bad timestamps.
So we introduce new y2038 safe timestamp options for 32 bit ABIs. We
assume that 32 bit applications will switch to new ABIs at some point,
but leave the older timestamps as is.
I can update the commit text as per above.
-Deepa
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply
* Re: [PATCH 7/8] socket: Add SO_TIMESTAMP[NS]_NEW
From: Deepa Dinamani @ 2018-11-25 5:55 UTC (permalink / raw)
To: willemdebruijn.kernel
Cc: open list:RALINK MIPS ARCHITECTURE, Parisc List, Arnd Bergmann,
y2038 Mailman List, Linux Network Devel Mailing List,
Linux Kernel Mailing List, Ralf Baechle, James E.J. Bottomley,
linux-rdma, Alexander Viro, linux-alpha, sparclinux,
David S. Miller, rth
In-Reply-To: <CABeXuvoDPvT45CCt2L0TReqtb1Q_-E8+toYu6qjiP0cxfdQNdA@mail.gmail.com>
A couple of comments I missed:
> > > > /*
> > > > * called from sock_recv_timestamp() if sock_flag(sk, SOCK_RCVTSTAMP)
> > > > * or sock_flag(sk, SOCK_RCVTSTAMPNS)
> > > > @@ -719,19 +751,8 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
> > > > false_tstamp = 1;
> > > > }
> > > > - if (need_software_tstamp) {
> > >
> > > Considerably less code churn if adding __sock_recv_timestamp_2038 and
> > > calling that here:
> > >
> > > if (sock_flag(sk, SOCK_TSTAMP_NEW))
> > > __sock_recv_timestamp_2038(msg, sk, skb);
> > > else if ...
> > >
> > > Same for the tcp case above, really, and in the case of the next patch
> > > for SO_TIMESTAMPING_NEW
> >
> > That naming convention, ..._2038, is not the nicest, of course. That
> > is not the relevant bit in the above comment.
it could be __sock_recv_timestamp64().
But, these timestamps should be doing exactly the same thing as the
old ones and I thought it would be nicer to keep the same code path.
I can change it to as per above.
> > Come to think of it, and related to my question in patch 2 why the
> > need to rename at all, could all new structs, constants and functions
> > be named consistently with 64 suffix? __sock_recv_timestamp64,
> > SO_TIMESTAMPING64 and timeval64 (instead of sock_timeval,
> > it isn't really a sock specific struct)?
> >
> > I guess that there is a good reason for the renaming exercise and
> > conditional mapping of SO_TIMESTAMP onto old or new interface.
> > Please elucidate in the commit message.
>
> I think there is some confusion here.
>
> The existing timestamp options: SO_TIMESTAMP* fail to provide proper
> timestamps beyond year 2038 on 32 bit ABIs.
> But, these work fine on 64 bit native ABIs.
> So now we need a way of updating these timestamps so that we do not
> break existing userspace: 64 bit ABIs should not have to change
> userspace, 32 bit ABIs should work as is until 2038 after which they
> have bad timestamps.
> So we introduce new y2038 safe timestamp options for 32 bit ABIs. We
> assume that 32 bit applications will switch to new ABIs at some point,
> but leave the older timestamps as is.
> I can update the commit text as per above.
We have been avoiding adding timeval64 timestamps to discourage users
from using these types in the interfaces.
We want to keep all the uapi time interfaces to use __kernel_*
interfaces. And, we already provide __kernel_timespec interface for
such instances.
But, in this case we do not have an option. So we introduce a type
specific to sockets.
-Deepa
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply
* Re: WARN_ON() in netconsole with PREEMPT_RT
From: Oleksandr Natalenko @ 2018-11-24 19:15 UTC (permalink / raw)
To: Steven Rostedt
Cc: David S. Miller, Sebastian Andrzej Siewior, Thomas Gleixner,
Eric Dumazet, Dave Jones, netdev, linux-rt-users, linux-kernel,
Nikolay Aleksandrov
In-Reply-To: <20181111210105.7f376c35@vmware.local.home>
Hi.
On 12.11.2018 03:01, Steven Rostedt wrote:
> On Sun, 11 Nov 2018 21:16:00 +0100
> Oleksandr Natalenko <oleksandr@natalenko.name> wrote:
>> Oh, I see that write_msg() calls netpoll_send_udp() under
>> spin_lock_irqsave(), but in PREEMPT_RT this, AFAIK, does not disable
>> interrupts.
>>
>> So, the real question here is whether the interrupts should be indeed
>> disabled. And if so, -rt should replace spin_lock_irqsave() call there
>> with what? local_irq_save()? and get rid of the warning?
>>
>
> Why do we care if interrupts are disabled with PREEMPT_RT here?
I don't know, if you ask me. But the check was added intentionally with
c9fd56b34efd0, so the author had definitely something on his mind.
Cc'ing him.
> printk() itself has a lot of issues with PREEMPT_RT that we are working
> on dealing with. Right now netconsole is actually at the end of that
> todo list.
OK, I see.
--
Oleksandr Natalenko (post-factum)
^ permalink raw reply
* [PATCH net] net: always initialize pagedlen
From: Willem de Bruijn @ 2018-11-24 19:21 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
In ip packet generation, pagedlen is initialized for each skb at the
start of the loop in __ip(6)_append_data, before label alloc_new_skb.
Depending on compiler options, code can be generated that jumps to
this label, triggering use of an an uninitialized variable.
In practice, at -O2, the generated code moves the initialization below
the label. But the code should not rely on that for correctness.
Fixes: 15e36f5b8e98 ("udp: paged allocation with gso")
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
Noticed when rebasing udp zerocopy. I considered merging as part of
that patchset, but this seemed like it should go to net, even if it
does not trigger in practice.
Merged net with this patch onto net-next with udp zerocopy to verify
that there is no merge conflict.
---
net/ipv4/ip_output.c | 3 ++-
net/ipv6/ip6_output.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index c09219e7f230..5dbec21856f4 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -939,7 +939,7 @@ static int __ip_append_data(struct sock *sk,
unsigned int fraglen;
unsigned int fraggap;
unsigned int alloclen;
- unsigned int pagedlen = 0;
+ unsigned int pagedlen;
struct sk_buff *skb_prev;
alloc_new_skb:
skb_prev = skb;
@@ -956,6 +956,7 @@ static int __ip_append_data(struct sock *sk,
if (datalen > mtu - fragheaderlen)
datalen = maxfraglen - fragheaderlen;
fraglen = datalen + fragheaderlen;
+ pagedlen = 0;
if ((flags & MSG_MORE) &&
!(rt->dst.dev->features&NETIF_F_SG))
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 89e0d5118afe..827a3f5ff3bb 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1354,7 +1354,7 @@ static int __ip6_append_data(struct sock *sk,
unsigned int fraglen;
unsigned int fraggap;
unsigned int alloclen;
- unsigned int pagedlen = 0;
+ unsigned int pagedlen;
alloc_new_skb:
/* There's no room in the current skb */
if (skb)
@@ -1378,6 +1378,7 @@ static int __ip6_append_data(struct sock *sk,
if (datalen > (cork->length <= mtu && !(cork->flags & IPCORK_ALLFRAG) ? mtu : maxfraglen) - fragheaderlen)
datalen = maxfraglen - fragheaderlen - rt->dst.trailer_len;
fraglen = datalen + fragheaderlen;
+ pagedlen = 0;
if ((flags & MSG_MORE) &&
!(rt->dst.dev->features&NETIF_F_SG))
--
2.20.0.rc0.387.gc7a69e6b6c-goog
^ permalink raw reply related
* [PATCH net-next 0/2] udp msg_zerocopy
From: Willem de Bruijn @ 2018-11-24 19:24 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
Enable MSG_ZEROCOPY for udp sockets
Patch 1/2 is the main patch, a rebase from RFC patch
http://patchwork.ozlabs.org/patch/899630/
more details in the patch commit message
Patch 2/2 runs the already existing udp zerocopy tests
as part of kselftest
See also recent Linux Plumbers presentation
https://linuxplumbersconf.org/event/2/contributions/106/attachments/104/128/willemdebruijn-lpc2018-udpgso-presentation-20181113.pdf
Willem de Bruijn (2):
udp: msg_zerocopy
tools/testing: extend zerocopy tests to udp
include/linux/skbuff.h | 1 +
net/core/skbuff.c | 15 +++++++++++++-
net/core/sock.c | 5 ++++-
net/ipv4/ip_output.c | 22 ++++++++++++++++++++-
net/ipv6/ip6_output.c | 22 ++++++++++++++++++++-
tools/testing/selftests/net/msg_zerocopy.c | 3 ++-
tools/testing/selftests/net/msg_zerocopy.sh | 2 ++
tools/testing/selftests/net/udpgso_bench.sh | 3 +++
8 files changed, 68 insertions(+), 5 deletions(-)
--
2.20.0.rc0.387.gc7a69e6b6c-goog
^ permalink raw reply
* [PATCH net-next 1/2] udp: msg_zerocopy
From: Willem de Bruijn @ 2018-11-24 19:24 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
In-Reply-To: <20181124192431.32037-1-willemdebruijn.kernel@gmail.com>
From: Willem de Bruijn <willemb@google.com>
Extend zerocopy to udp sockets. Allow setting sockopt SO_ZEROCOPY and
interpret flag MSG_ZEROCOPY.
This patch was previously part of the zerocopy RFC patchsets. Zerocopy
is not effective at small MTU. With segmentation offload building
larger datagrams, the benefit of page flipping outweights the cost of
generating a completion notification.
The datagram implementation has initial refcnt of 0, instead of 1 for
TCP. The tcp_sendmsg_locked function has to hold a reference on
uarg independent those held by the skbs it generates, because the
skbs can be sent and freed in the function main loop. This is not
needed for other sockets.
tools/testing/selftests/net/msg_zerocopy.sh after applying follow-on
test patch and making skb_orphan_frags_rx same as skb_orphan_frags:
ipv4 udp -t 1
tx=191312 (11938 MB) txc=0 zc=n
rx=191312 (11938 MB)
ipv4 udp -z -t 1
tx=304507 (19002 MB) txc=304507 zc=y
rx=304507 (19002 MB)
ok
ipv6 udp -t 1
tx=174485 (10888 MB) txc=0 zc=n
rx=174485 (10888 MB)
ipv6 udp -z -t 1
tx=294801 (18396 MB) txc=294801 zc=y
rx=294801 (18396 MB)
ok
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
include/linux/skbuff.h | 1 +
net/core/skbuff.c | 15 ++++++++++++++-
net/core/sock.c | 5 ++++-
net/ipv4/ip_output.c | 22 +++++++++++++++++++++-
net/ipv6/ip6_output.c | 22 +++++++++++++++++++++-
5 files changed, 61 insertions(+), 4 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index a2e8297a5b00..7c1a649d229b 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -485,6 +485,7 @@ void sock_zerocopy_put_abort(struct ubuf_info *uarg);
void sock_zerocopy_callback(struct ubuf_info *uarg, bool success);
+int skb_zerocopy_iter_dgram(struct sk_buff *skb, struct msghdr *msg, int len);
int skb_zerocopy_iter_stream(struct sock *sk, struct sk_buff *skb,
struct msghdr *msg, int len,
struct ubuf_info *uarg);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 9a8a72cefe9b..b7f8894fbcb4 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -957,7 +957,7 @@ struct ubuf_info *sock_zerocopy_alloc(struct sock *sk, size_t size)
uarg->len = 1;
uarg->bytelen = size;
uarg->zerocopy = 1;
- refcount_set(&uarg->refcnt, 1);
+ refcount_set(&uarg->refcnt, sk->sk_type == SOCK_STREAM ? 1 : 0);
sock_hold(sk);
return uarg;
@@ -1097,6 +1097,13 @@ void sock_zerocopy_put_abort(struct ubuf_info *uarg)
atomic_dec(&sk->sk_zckey);
uarg->len--;
+ /* Stream socks hold a ref for the syscall, as skbs can be sent
+ * and freed inside the loop, dropping refcnt to 0 inbetween.
+ * Datagrams do not need this, but sock_zerocopy_put expects it.
+ */
+ if (sk->sk_type != SOCK_STREAM && !refcount_read(&uarg->refcnt))
+ refcount_set(&uarg->refcnt, 1);
+
sock_zerocopy_put(uarg);
}
}
@@ -1105,6 +1112,12 @@ EXPORT_SYMBOL_GPL(sock_zerocopy_put_abort);
extern int __zerocopy_sg_from_iter(struct sock *sk, struct sk_buff *skb,
struct iov_iter *from, size_t length);
+int skb_zerocopy_iter_dgram(struct sk_buff *skb, struct msghdr *msg, int len)
+{
+ return __zerocopy_sg_from_iter(skb->sk, skb, &msg->msg_iter, len);
+}
+EXPORT_SYMBOL_GPL(skb_zerocopy_iter_dgram);
+
int skb_zerocopy_iter_stream(struct sock *sk, struct sk_buff *skb,
struct msghdr *msg, int len,
struct ubuf_info *uarg)
diff --git a/net/core/sock.c b/net/core/sock.c
index 6d7e189e3cd9..f5bb89785e47 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1018,7 +1018,10 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
case SO_ZEROCOPY:
if (sk->sk_family == PF_INET || sk->sk_family == PF_INET6) {
- if (sk->sk_protocol != IPPROTO_TCP)
+ if (!((sk->sk_type == SOCK_STREAM &&
+ sk->sk_protocol == IPPROTO_TCP) ||
+ (sk->sk_type == SOCK_DGRAM &&
+ sk->sk_protocol == IPPROTO_UDP)))
ret = -ENOTSUPP;
} else if (sk->sk_family != PF_RDS) {
ret = -ENOTSUPP;
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index c09219e7f230..2d1e4b67a1e8 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -868,6 +868,7 @@ static int __ip_append_data(struct sock *sk,
{
struct inet_sock *inet = inet_sk(sk);
struct sk_buff *skb;
+ struct ubuf_info *uarg = NULL;
struct ip_options *opt = cork->opt;
int hh_len;
@@ -916,6 +917,19 @@ static int __ip_append_data(struct sock *sk,
(!exthdrlen || (rt->dst.dev->features & NETIF_F_HW_ESP_TX_CSUM)))
csummode = CHECKSUM_PARTIAL;
+ if (flags & MSG_ZEROCOPY && length) {
+ uarg = sock_zerocopy_realloc(sk, length, skb_zcopy(skb));
+ if (!uarg)
+ return -ENOBUFS;
+ if (rt->dst.dev->features & NETIF_F_SG &&
+ csummode == CHECKSUM_PARTIAL) {
+ paged = true;
+ } else {
+ uarg->zerocopy = 0;
+ skb_zcopy_set(skb, uarg);
+ }
+ }
+
cork->length += length;
/* So, what's going on in the loop below?
@@ -1005,6 +1019,7 @@ static int __ip_append_data(struct sock *sk,
cork->tx_flags = 0;
skb_shinfo(skb)->tskey = tskey;
tskey = 0;
+ skb_zcopy_set(skb, uarg);
/*
* Find where to start putting bytes.
@@ -1067,7 +1082,7 @@ static int __ip_append_data(struct sock *sk,
err = -EFAULT;
goto error;
}
- } else {
+ } else if (!uarg || !uarg->zerocopy) {
int i = skb_shinfo(skb)->nr_frags;
err = -ENOMEM;
@@ -1097,6 +1112,10 @@ static int __ip_append_data(struct sock *sk,
skb->data_len += copy;
skb->truesize += copy;
wmem_alloc_delta += copy;
+ } else {
+ err = skb_zerocopy_iter_dgram(skb, from, copy);
+ if (err < 0)
+ goto error;
}
offset += copy;
length -= copy;
@@ -1109,6 +1128,7 @@ static int __ip_append_data(struct sock *sk,
error_efault:
err = -EFAULT;
error:
+ sock_zerocopy_put_abort(uarg);
cork->length -= length;
IP_INC_STATS(sock_net(sk), IPSTATS_MIB_OUTDISCARDS);
refcount_add(wmem_alloc_delta, &sk->sk_wmem_alloc);
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 89e0d5118afe..8a49ff985ed8 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1245,6 +1245,7 @@ static int __ip6_append_data(struct sock *sk,
{
struct sk_buff *skb, *skb_prev = NULL;
unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu, pmtu;
+ struct ubuf_info *uarg = NULL;
int exthdrlen = 0;
int dst_exthdrlen = 0;
int hh_len;
@@ -1322,6 +1323,19 @@ static int __ip6_append_data(struct sock *sk,
rt->dst.dev->features & (NETIF_F_IPV6_CSUM | NETIF_F_HW_CSUM))
csummode = CHECKSUM_PARTIAL;
+ if (flags & MSG_ZEROCOPY && length) {
+ uarg = sock_zerocopy_realloc(sk, length, skb_zcopy(skb));
+ if (!uarg)
+ return -ENOBUFS;
+ if (rt->dst.dev->features & NETIF_F_SG &&
+ csummode == CHECKSUM_PARTIAL) {
+ paged = true;
+ } else {
+ uarg->zerocopy = 0;
+ skb_zcopy_set(skb, uarg);
+ }
+ }
+
/*
* Let's try using as much space as possible.
* Use MTU if total length of the message fits into the MTU.
@@ -1443,6 +1457,7 @@ static int __ip6_append_data(struct sock *sk,
skb_shinfo(skb)->tx_flags = cork->tx_flags;
cork->tx_flags = 0;
skb_shinfo(skb)->tskey = tskey;
+ skb_zcopy_set(skb, uarg);
tskey = 0;
/*
@@ -1505,7 +1520,7 @@ static int __ip6_append_data(struct sock *sk,
err = -EFAULT;
goto error;
}
- } else {
+ } else if (!uarg || !uarg->zerocopy) {
int i = skb_shinfo(skb)->nr_frags;
err = -ENOMEM;
@@ -1535,6 +1550,10 @@ static int __ip6_append_data(struct sock *sk,
skb->data_len += copy;
skb->truesize += copy;
wmem_alloc_delta += copy;
+ } else {
+ err = skb_zerocopy_iter_dgram(skb, from, copy);
+ if (err < 0)
+ goto error;
}
offset += copy;
length -= copy;
@@ -1547,6 +1566,7 @@ static int __ip6_append_data(struct sock *sk,
error_efault:
err = -EFAULT;
error:
+ sock_zerocopy_put_abort(uarg);
cork->length -= length;
IP6_INC_STATS(sock_net(sk), rt->rt6i_idev, IPSTATS_MIB_OUTDISCARDS);
refcount_add(wmem_alloc_delta, &sk->sk_wmem_alloc);
--
2.20.0.rc0.387.gc7a69e6b6c-goog
^ permalink raw reply related
* [PATCH net-next 2/2] tools/testing: extend zerocopy tests to udp
From: Willem de Bruijn @ 2018-11-24 19:24 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
In-Reply-To: <20181124192431.32037-1-willemdebruijn.kernel@gmail.com>
From: Willem de Bruijn <willemb@google.com>
Both msg_zerocopy and udpgso_bench have udp zerocopy variants.
Exercise these as part of the standard kselftest run.
With udp, msg_zerocopy has no control channel. Ensure that the
receiver exits after the sender by accounting for the initial
delay in starting them (in msg_zerocopy.sh).
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
tools/testing/selftests/net/msg_zerocopy.c | 3 ++-
tools/testing/selftests/net/msg_zerocopy.sh | 2 ++
tools/testing/selftests/net/udpgso_bench.sh | 3 +++
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/msg_zerocopy.c b/tools/testing/selftests/net/msg_zerocopy.c
index 406cc70c571d..4b02933cab8a 100644
--- a/tools/testing/selftests/net/msg_zerocopy.c
+++ b/tools/testing/selftests/net/msg_zerocopy.c
@@ -651,12 +651,13 @@ static void do_flush_datagram(int fd, int type)
static void do_rx(int domain, int type, int protocol)
{
+ const int cfg_receiver_wait_ms = 400;
uint64_t tstop;
int fd;
fd = do_setup_rx(domain, type, protocol);
- tstop = gettimeofday_ms() + cfg_runtime_ms;
+ tstop = gettimeofday_ms() + cfg_runtime_ms + cfg_receiver_wait_ms;
do {
if (type == SOCK_STREAM)
do_flush_tcp(fd);
diff --git a/tools/testing/selftests/net/msg_zerocopy.sh b/tools/testing/selftests/net/msg_zerocopy.sh
index c43c6debda06..825ffec85cea 100755
--- a/tools/testing/selftests/net/msg_zerocopy.sh
+++ b/tools/testing/selftests/net/msg_zerocopy.sh
@@ -25,6 +25,8 @@ readonly path_sysctl_mem="net.core.optmem_max"
if [[ "$#" -eq "0" ]]; then
$0 4 tcp -t 1
$0 6 tcp -t 1
+ $0 4 udp -t 1
+ $0 6 udp -t 1
echo "OK. All tests passed"
exit 0
fi
diff --git a/tools/testing/selftests/net/udpgso_bench.sh b/tools/testing/selftests/net/udpgso_bench.sh
index 0f0628613f81..5670a9ffd8eb 100755
--- a/tools/testing/selftests/net/udpgso_bench.sh
+++ b/tools/testing/selftests/net/udpgso_bench.sh
@@ -35,6 +35,9 @@ run_udp() {
echo "udp gso"
run_in_netns ${args} -S 0
+
+ echo "udp gso zerocopy"
+ run_in_netns ${args} -S 0 -z
}
run_tcp() {
--
2.20.0.rc0.387.gc7a69e6b6c-goog
^ permalink raw reply related
* Re: WARNING in csum_and_copy_to_iter
From: Al Viro @ 2018-11-24 20:03 UTC (permalink / raw)
To: syzbot
Cc: davem, gregkh, kgraul, linux-kernel, netdev, stranche,
syzkaller-bugs
In-Reply-To: <0000000000001ecaa1057b6e4489@google.com>
On Sat, Nov 24, 2018 at 11:40:03AM -0800, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: edeca3a769ad Merge tag 'sound-4.20-rc4' of git://git.kerne..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=12bee26d400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=73e2bc0cb6463446
> dashboard link: https://syzkaller.appspot.com/bug?extid=ce18da013d76d837144d
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15ccd1f5400000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+ce18da013d76d837144d@syzkaller.appspotmail.com
Caused by commit 95506588d2c1d72ca29adef8ae9bf771bcfb4ced
Author: Slavomir Kaslev <kaslevs@vmware.com>
Date: Fri Nov 16 11:27:53 2018 +0200
socket: do a generic_file_splice_read when proto_ops has no splice_read
exposing all ->recvmsg() instances to pipe-backed iov_iter as possible destination.
It's not all that hard to fix (I'll probably have a candidate patch by tonight,
it's just a matter of adding the only missing primitive), but... shouldn't that
patch have sat in -next for at least some testing first? Because it's very
easy to reproduce - splice from e.g. UDP socket will step into it. Sure, the
sky is not falling (unless you set panic-on-WARN, that is); the damn thing
would've failed anyway, but...
^ permalink raw reply
* Re: [PATCH] brcmfmac: Fix out of bounds memory access during fw load
From: Kalle Valo @ 2018-11-25 8:15 UTC (permalink / raw)
To: Lyude Paul
Cc: brcm80211-dev-list, Hante Meuleman, Pieter-Paul Giesberts,
Franky Lin, Arend van Spriel, Himanshu Jha, Dan Haab,
Jia-Shyr Chuang, Ian Molton, stable, Chi-Hsien Lin, Wright Feng,
David S. Miller, linux-wireless, brcm80211-dev-list.pdl, netdev,
linux-kernel
In-Reply-To: <20181124225707.12262-1-lyude@redhat.com>
Lyude Paul <lyude@redhat.com> writes:
> I ended up tracking down some rather nasty issues with f2fs (and other
> filesystem modules) constantly crashing on my kernel down to a
> combination of out of bounds memory accesses, one of which was coming
> from brcmfmac during module load:
>
> [ 30.891382] brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac4356-sdio for chip BCM4356/2
> [ 30.894437] ==================================================================
> [ 30.901581] BUG: KASAN: global-out-of-bounds in brcmf_fw_alloc_request+0x42c/0x480 [brcmfmac]
> [ 30.909935] Read of size 1 at addr ffff2000024865df by task kworker/6:2/387
> [ 30.916805]
> [ 30.918261] CPU: 6 PID: 387 Comm: kworker/6:2 Tainted: G O 4.20.0-rc3Lyude-Test+ #19
> [ 30.927251] Hardware name: amlogic khadas-vim2/khadas-vim2, BIOS 2018.07-rc2-armbian 09/11/2018
> [ 30.935964] Workqueue: events brcmf_driver_register [brcmfmac]
> [ 30.941641] Call trace:
> [ 30.944058] dump_backtrace+0x0/0x3e8
> [ 30.947676] show_stack+0x14/0x20
> [ 30.950968] dump_stack+0x130/0x1c4
> [ 30.954406] print_address_description+0x60/0x25c
> [ 30.959066] kasan_report+0x1b4/0x368
> [ 30.962683] __asan_report_load1_noabort+0x18/0x20
> [ 30.967547] brcmf_fw_alloc_request+0x42c/0x480 [brcmfmac]
> [ 30.967639] brcmf_sdio_probe+0x163c/0x2050 [brcmfmac]
> [ 30.978035] brcmf_ops_sdio_probe+0x598/0xa08 [brcmfmac]
> [ 30.983254] sdio_bus_probe+0x190/0x398
> [ 30.983270] really_probe+0x2a0/0xa70
> [ 30.983296] driver_probe_device+0x1b4/0x2d8
> [ 30.994901] __driver_attach+0x200/0x280
> [ 30.994914] bus_for_each_dev+0x10c/0x1a8
> [ 30.994925] driver_attach+0x38/0x50
> [ 30.994935] bus_add_driver+0x330/0x608
> [ 30.994953] driver_register+0x140/0x388
> [ 31.013965] sdio_register_driver+0x74/0xa0
> [ 31.014076] brcmf_sdio_register+0x14/0x60 [brcmfmac]
> [ 31.023177] brcmf_driver_register+0xc/0x18 [brcmfmac]
> [ 31.023209] process_one_work+0x654/0x1080
> [ 31.032266] worker_thread+0x4f0/0x1308
> [ 31.032286] kthread+0x2a8/0x320
> [ 31.039254] ret_from_fork+0x10/0x1c
> [ 31.039269]
> [ 31.044226] The buggy address belongs to the variable:
> [ 31.044351] brcmf_firmware_path+0x11f/0xfffffffffffd3b40 [brcmfmac]
> [ 31.055601]
> [ 31.057031] Memory state around the buggy address:
> [ 31.061800] ffff200002486480: 04 fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
> [ 31.068983] ffff200002486500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 31.068993] >ffff200002486580: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
> [ 31.068999] ^
> [ 31.069017] ffff200002486600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 31.096521] ffff200002486680: 00 00 00 00 00 00 00 00 00 00 00 00 fa fa fa fa
> [ 31.096528] ==================================================================
> [ 31.096533] Disabling lock debugging due to kernel taint
>
> It appears that when trying to determine the length of the string in the
> alternate firmware path, we make the mistake of not handling the case
> where the firmware path is empty correctly. Since strlen(mp_path) can
> return 0, we'll end up accessing mp_path[-1] when the firmware_path
> isn't provided through the module arguments.
>
> So, fix this by just setting the end char to '\0' by default, and only
> changing it if we have a non-zero length. Additionally, use strnlen()
> with BRCMF_FW_ALTPATH_LEN instead of strlen() just to be extra safe.
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
>
> Fixes: 2baa3aaee27f ("brcmfmac: introduce brcmf_fw_alloc_request() function")
> Cc: Hante Meuleman <hante.meuleman@broadcom.com>
> Cc: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
> Cc: Franky Lin <franky.lin@broadcom.com>
> Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: Arend Van Spriel <arend.vanspriel@broadcom.com>
> Cc: Himanshu Jha <himanshujha199640@gmail.com>
> Cc: Dan Haab <dhaab@luxul.com>
> Cc: Jia-Shyr Chuang <saint.chuang@cypress.com>
> Cc: Ian Molton <ian@mnementh.co.uk>
> Cc: <stable@vger.kernel.org> # v4.17+
> Signed-off-by: Lyude Paul <lyude@redhat.com>
Lyude's s-o-b is twice, I can fix that.
Arend, should I queue this to 4.20?
--
Kalle Valo
^ permalink raw reply
* Re: WARNING in csum_and_copy_to_iter
From: Al Viro @ 2018-11-24 21:44 UTC (permalink / raw)
To: Slavomir Kaslev
Cc: syzbot, davem, gregkh, kgraul, linux-kernel, netdev, stranche,
syzkaller-bugs
In-Reply-To: <20181124211918.GA20435@box>
On Sat, Nov 24, 2018 at 11:20:14PM +0200, Slavomir Kaslev wrote:
> On Sat, Nov 24, 2018 at 08:03:57PM +0000, Al Viro wrote:
> > On Sat, Nov 24, 2018 at 11:40:03AM -0800, syzbot wrote:
> > > Hello,
> > >
> > > syzbot found the following crash on:
> > >
> > > HEAD commit: edeca3a769ad Merge tag 'sound-4.20-rc4' of git://git.kerne..
> > > git tree: upstream
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=12bee26d400000
> > > kernel config: https://syzkaller.appspot.com/x/.config?x=73e2bc0cb6463446
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=ce18da013d76d837144d
> > > compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15ccd1f5400000
> > >
> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > Reported-by: syzbot+ce18da013d76d837144d@syzkaller.appspotmail.com
> >
> > Caused by commit 95506588d2c1d72ca29adef8ae9bf771bcfb4ced
> > Author: Slavomir Kaslev <kaslevs@vmware.com>
> > Date: Fri Nov 16 11:27:53 2018 +0200
> >
> > socket: do a generic_file_splice_read when proto_ops has no splice_read
> >
> > exposing all ->recvmsg() instances to pipe-backed iov_iter as possible destination.
> > It's not all that hard to fix (I'll probably have a candidate patch by tonight,
> > it's just a matter of adding the only missing primitive), but... shouldn't that
> > patch have sat in -next for at least some testing first? Because it's very
> > easy to reproduce - splice from e.g. UDP socket will step into it. Sure, the
> > sky is not falling (unless you set panic-on-WARN, that is); the damn thing
> > would've failed anyway, but...
>
> My bad for not sending the patch tagged as net-next, feel free to revert it.
No point, IMO - the fix isn't hard and bisect hazard created by the whole thing
is both mild (spurious WARN() in case that used to fail anyway) _and_ won't
disappear from reverting, obviously. I'll post a fix later tonight...
^ permalink raw reply
* Re: [PATCH net-next 1/8] dpaa2-eth: Add basic XDP support
From: David Ahern @ 2018-11-24 21:49 UTC (permalink / raw)
To: Ioana Ciocoi Radulescu, netdev@vger.kernel.org,
davem@davemloft.net
Cc: Ioana Ciornei
In-Reply-To: <1542992186-26028-2-git-send-email-ruxandra.radulescu@nxp.com>
On 11/23/18 9:56 AM, Ioana Ciocoi Radulescu wrote:
> @@ -215,6 +255,7 @@ static void dpaa2_eth_rx(struct dpaa2_eth_priv *priv,
> struct dpaa2_fas *fas;
> void *buf_data;
> u32 status = 0;
> + u32 xdp_act;
>
> /* Tracing point */
> trace_dpaa2_rx_fd(priv->net_dev, fd);
> @@ -231,8 +272,14 @@ static void dpaa2_eth_rx(struct dpaa2_eth_priv *priv,
> percpu_extras = this_cpu_ptr(priv->percpu_extras);
>
> if (fd_format == dpaa2_fd_single) {
> + xdp_act = run_xdp(priv, ch, (struct dpaa2_fd *)fd, vaddr);
> + if (xdp_act != XDP_PASS)
> + return;
please bump the rx counters (packets and bytes) regardless of what XDP
outcome is.
Same for Tx; packets and bytes counter should be bumped for packets
redirected by XDP.
^ permalink raw reply
* Re: [PATCH v3 3/4] libbpf: add bpf_prog_test_run_xattr
From: Alexei Starovoitov @ 2018-11-24 22:20 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Lorenz Bauer, ast, netdev, linux-api, ys114321
In-Reply-To: <b4308b6f-5a1d-f015-4e7f-abca1b3902be@iogearbox.net>
On Fri, Nov 23, 2018 at 11:25:11PM +0100, Daniel Borkmann wrote:
> On 11/22/2018 03:09 PM, Lorenz Bauer wrote:
> > Add a new function, which encourages safe usage of the test interface.
> > bpf_prog_test_run continues to work as before, but should be considered
> > unsafe.
> >
> > Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
>
> Set looks good to me, thanks! Three small things below:
>
> > ---
> > tools/lib/bpf/bpf.c | 27 +++++++++++++++++++++++++++
> > tools/lib/bpf/bpf.h | 13 +++++++++++++
> > 2 files changed, 40 insertions(+)
> >
> > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> > index 961e1b9fc592..f8518bef6886 100644
> > --- a/tools/lib/bpf/bpf.c
> > +++ b/tools/lib/bpf/bpf.c
> > @@ -424,6 +424,33 @@ int bpf_prog_test_run(int prog_fd, int repeat, void *data, __u32 size,
> > return ret;
> > }
> >
> > +int bpf_prog_test_run_xattr(const struct bpf_prog_test_run_attr *test_attr,
> > + __u32 *size_out, __u32 *retval, __u32 *duration)
> > +{
> > + union bpf_attr attr;
> > + int ret;
> > +
> > + if (!test_attr->data_out && test_attr->size_out > 0)
> > + return -EINVAL;
> > +
> > + bzero(&attr, sizeof(attr));
> > + attr.test.prog_fd = test_attr->prog_fd;
> > + attr.test.data_in = ptr_to_u64(test_attr->data);
> > + attr.test.data_out = ptr_to_u64(test_attr->data_out);
> > + attr.test.data_size_in = test_attr->size;
> > + attr.test.data_size_out = test_attr->size_out;
> > + attr.test.repeat = test_attr->repeat;
> > +
> > + ret = sys_bpf(BPF_PROG_TEST_RUN, &attr, sizeof(attr));
> > + if (size_out)
> > + *size_out = attr.test.data_size_out;
> > + if (retval)
> > + *retval = attr.test.retval;
> > + if (duration)
> > + *duration = attr.test.duration;
> > + return ret;
> > +}
> > +
> > int bpf_prog_get_next_id(__u32 start_id, __u32 *next_id)
> > {
> > union bpf_attr attr;
> > diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> > index 26a51538213c..570f19f77f42 100644
> > --- a/tools/lib/bpf/bpf.h
> > +++ b/tools/lib/bpf/bpf.h
> > @@ -110,6 +110,19 @@ LIBBPF_API int bpf_prog_attach(int prog_fd, int attachable_fd,
> > LIBBPF_API int bpf_prog_detach(int attachable_fd, enum bpf_attach_type type);
> > LIBBPF_API int bpf_prog_detach2(int prog_fd, int attachable_fd,
> > enum bpf_attach_type type);
> > +
> > +struct bpf_prog_test_run_attr {
> > + int prog_fd;
> > + int repeat;
> > + const void *data;
> > + __u32 size;
> > + void *data_out; /* optional */
> > + __u32 size_out;
>
> Small nit: could we name these data_{in,out} and data_size_{in,out} as
> well, so it's analog to the ones from the bpf_attr?
>
> > +};
> > +
> > +LIBBPF_API int bpf_prog_test_run_xattr(const struct bpf_prog_test_run_attr *test_attr,
> > + __u32 *size_out, __u32 *retval,
> > + __u32 *duration);
can we keep return values in struct bpf_prog_test_run_attr ?
I think the interface will be easier to use and faster. Less pointers
to pass around.
> > LIBBPF_API int bpf_prog_test_run(int prog_fd, int repeat, void *data,
> > __u32 size, void *data_out, __u32 *size_out,
> > __u32 *retval, __u32 *duration);
>
> Could we add a comment into the header here stating that we discourage
> bpf_prog_test_run()'s use?
>
> It would probably also make sense since we go that route that we would
> convert the 10 bpf_prog_test_run() instances under test_progs.c at the
> same time so that people extending or looking at BPF kselftests don't
> copy discouraged bpf_prog_test_run() api as examples from this point
> onwards anymore.
I would keep bpf_prog_test_run() and test_progs.c as-is.
I think the prog_test_run in the current form is actually fine.
I don't find it 'unsafe'.
When it's called on a given bpf prog the user knows what prog
suppose to do. If prog is increasing the packet size the test writer
knows that and should size the output buffer accordingly.
Also there are plenty of tests where progs don't change the packet size
and any test with 'repeat > 1' should have the packet size
return to initial value. Like if the test is doing ipip encap
at the end of the run the bpf prog should remove that encap.
Otherwise 'repeat > 1' will produce wrong results.
^ permalink raw reply
* [PATCH] brcmfmac: Fix out of bounds memory access during fw load
From: Lyude Paul @ 2018-11-24 22:57 UTC (permalink / raw)
To: brcm80211-dev-list
Cc: Hante Meuleman, Pieter-Paul Giesberts, Franky Lin,
Arend van Spriel, Kalle Valo, Himanshu Jha, Dan Haab,
Jia-Shyr Chuang, Ian Molton, stable, Chi-Hsien Lin, Wright Feng,
David S. Miller, linux-wireless, brcm80211-dev-list.pdl, netdev,
linux-kernel
I ended up tracking down some rather nasty issues with f2fs (and other
filesystem modules) constantly crashing on my kernel down to a
combination of out of bounds memory accesses, one of which was coming
from brcmfmac during module load:
[ 30.891382] brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac4356-sdio for chip BCM4356/2
[ 30.894437] ==================================================================
[ 30.901581] BUG: KASAN: global-out-of-bounds in brcmf_fw_alloc_request+0x42c/0x480 [brcmfmac]
[ 30.909935] Read of size 1 at addr ffff2000024865df by task kworker/6:2/387
[ 30.916805]
[ 30.918261] CPU: 6 PID: 387 Comm: kworker/6:2 Tainted: G O 4.20.0-rc3Lyude-Test+ #19
[ 30.927251] Hardware name: amlogic khadas-vim2/khadas-vim2, BIOS 2018.07-rc2-armbian 09/11/2018
[ 30.935964] Workqueue: events brcmf_driver_register [brcmfmac]
[ 30.941641] Call trace:
[ 30.944058] dump_backtrace+0x0/0x3e8
[ 30.947676] show_stack+0x14/0x20
[ 30.950968] dump_stack+0x130/0x1c4
[ 30.954406] print_address_description+0x60/0x25c
[ 30.959066] kasan_report+0x1b4/0x368
[ 30.962683] __asan_report_load1_noabort+0x18/0x20
[ 30.967547] brcmf_fw_alloc_request+0x42c/0x480 [brcmfmac]
[ 30.967639] brcmf_sdio_probe+0x163c/0x2050 [brcmfmac]
[ 30.978035] brcmf_ops_sdio_probe+0x598/0xa08 [brcmfmac]
[ 30.983254] sdio_bus_probe+0x190/0x398
[ 30.983270] really_probe+0x2a0/0xa70
[ 30.983296] driver_probe_device+0x1b4/0x2d8
[ 30.994901] __driver_attach+0x200/0x280
[ 30.994914] bus_for_each_dev+0x10c/0x1a8
[ 30.994925] driver_attach+0x38/0x50
[ 30.994935] bus_add_driver+0x330/0x608
[ 30.994953] driver_register+0x140/0x388
[ 31.013965] sdio_register_driver+0x74/0xa0
[ 31.014076] brcmf_sdio_register+0x14/0x60 [brcmfmac]
[ 31.023177] brcmf_driver_register+0xc/0x18 [brcmfmac]
[ 31.023209] process_one_work+0x654/0x1080
[ 31.032266] worker_thread+0x4f0/0x1308
[ 31.032286] kthread+0x2a8/0x320
[ 31.039254] ret_from_fork+0x10/0x1c
[ 31.039269]
[ 31.044226] The buggy address belongs to the variable:
[ 31.044351] brcmf_firmware_path+0x11f/0xfffffffffffd3b40 [brcmfmac]
[ 31.055601]
[ 31.057031] Memory state around the buggy address:
[ 31.061800] ffff200002486480: 04 fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
[ 31.068983] ffff200002486500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 31.068993] >ffff200002486580: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
[ 31.068999] ^
[ 31.069017] ffff200002486600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 31.096521] ffff200002486680: 00 00 00 00 00 00 00 00 00 00 00 00 fa fa fa fa
[ 31.096528] ==================================================================
[ 31.096533] Disabling lock debugging due to kernel taint
It appears that when trying to determine the length of the string in the
alternate firmware path, we make the mistake of not handling the case
where the firmware path is empty correctly. Since strlen(mp_path) can
return 0, we'll end up accessing mp_path[-1] when the firmware_path
isn't provided through the module arguments.
So, fix this by just setting the end char to '\0' by default, and only
changing it if we have a non-zero length. Additionally, use strnlen()
with BRCMF_FW_ALTPATH_LEN instead of strlen() just to be extra safe.
Signed-off-by: Lyude Paul <lyude@redhat.com>
Fixes: 2baa3aaee27f ("brcmfmac: introduce brcmf_fw_alloc_request() function")
Cc: Hante Meuleman <hante.meuleman@broadcom.com>
Cc: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Cc: Franky Lin <franky.lin@broadcom.com>
Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: Arend Van Spriel <arend.vanspriel@broadcom.com>
Cc: Himanshu Jha <himanshujha199640@gmail.com>
Cc: Dan Haab <dhaab@luxul.com>
Cc: Jia-Shyr Chuang <saint.chuang@cypress.com>
Cc: Ian Molton <ian@mnementh.co.uk>
Cc: <stable@vger.kernel.org> # v4.17+
Signed-off-by: Lyude Paul <lyude@redhat.com>
---
.../net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 9095b830ae4d..9927079a9ace 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -641,8 +641,9 @@ brcmf_fw_alloc_request(u32 chip, u32 chiprev,
struct brcmf_fw_request *fwreq;
char chipname[12];
const char *mp_path;
+ size_t mp_path_len;
u32 i, j;
- char end;
+ char end = '\0';
size_t reqsz;
for (i = 0; i < table_size; i++) {
@@ -667,7 +668,10 @@ brcmf_fw_alloc_request(u32 chip, u32 chiprev,
mapping_table[i].fw_base, chipname);
mp_path = brcmf_mp_global.firmware_path;
- end = mp_path[strlen(mp_path) - 1];
+ mp_path_len = strnlen(mp_path, BRCMF_FW_ALTPATH_LEN);
+ if (mp_path_len)
+ end = mp_path[mp_path_len - 1];
+
fwreq->n_items = n_fwnames;
for (j = 0; j < n_fwnames; j++) {
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 0/2] r8169: make use of xmit_more and __netdev_sent_queue
From: Heiner Kallweit @ 2018-11-24 23:12 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers; +Cc: netdev@vger.kernel.org
This series adds helper __netdev_sent_queue to the core and makes use
of it in the r8169 driver.
Heiner Kallweit (2):
net: core: add __netdev_sent_queue as variant of __netdev_tx_sent_queue
r8169: make use of xmit_more and __netdev_sent_queue
drivers/net/ethernet/realtek/r8169.c | 19 +++++++++----------
include/linux/netdevice.h | 8 ++++++++
2 files changed, 17 insertions(+), 10 deletions(-)
--
2.19.1
^ permalink raw reply
* [PATCH net-next 1/2] net: core: add __netdev_sent_queue as variant of __netdev_tx_sent_queue
From: Heiner Kallweit @ 2018-11-24 23:13 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers; +Cc: netdev@vger.kernel.org
In-Reply-To: <e30bd939-f16f-3560-9109-9027f45e2175@gmail.com>
Similar to netdev_sent_queue add helper __netdev_sent_queue as variant
of __netdev_tx_sent_queue.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
include/linux/netdevice.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 1dcc0628b..a417fa501 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3214,6 +3214,14 @@ static inline void netdev_sent_queue(struct net_device *dev, unsigned int bytes)
netdev_tx_sent_queue(netdev_get_tx_queue(dev, 0), bytes);
}
+static inline bool __netdev_sent_queue(struct net_device *dev,
+ unsigned int bytes,
+ bool xmit_more)
+{
+ return __netdev_tx_sent_queue(netdev_get_tx_queue(dev, 0), bytes,
+ xmit_more);
+}
+
static inline void netdev_tx_completed_queue(struct netdev_queue *dev_queue,
unsigned int pkts, unsigned int bytes)
{
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 2/2] r8169: make use of xmit_more and __netdev_sent_queue
From: Heiner Kallweit @ 2018-11-24 23:14 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers; +Cc: netdev@vger.kernel.org
In-Reply-To: <e30bd939-f16f-3560-9109-9027f45e2175@gmail.com>
Make use of xmit_more and add the functionality introduced with
3e59020abf0f ("net: bql: add __netdev_tx_sent_queue()").
I used the mlx4 driver as template.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 5ee684f9e..7a979c1e5 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -6070,6 +6070,7 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
dma_addr_t mapping;
u32 opts[2], len;
int frags;
+ bool stop_queue;
if (unlikely(!rtl_tx_slots_avail(tp, skb_shinfo(skb)->nr_frags))) {
netif_err(tp, drv, dev, "BUG! Tx Ring full when queue awake!\n");
@@ -6110,8 +6111,6 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
txd->opts2 = cpu_to_le32(opts[1]);
- netdev_sent_queue(dev, skb->len);
-
skb_tx_timestamp(skb);
/* Force memory writes to complete before releasing descriptor */
@@ -6124,16 +6123,16 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
tp->cur_tx += frags + 1;
- RTL_W8(tp, TxPoll, NPQ);
+ stop_queue = !rtl_tx_slots_avail(tp, MAX_SKB_FRAGS);
+ if (unlikely(stop_queue))
+ netif_stop_queue(dev);
- mmiowb();
+ if (__netdev_sent_queue(dev, skb->len, skb->xmit_more)) {
+ RTL_W8(tp, TxPoll, NPQ);
+ mmiowb();
+ }
- if (!rtl_tx_slots_avail(tp, MAX_SKB_FRAGS)) {
- /* Avoid wrongly optimistic queue wake-up: rtl_tx thread must
- * not miss a ring update when it notices a stopped queue.
- */
- smp_wmb();
- netif_stop_queue(dev);
+ if (unlikely(stop_queue)) {
/* Sync with rtl_tx:
* - publish queue status and cur_tx ring index (write barrier)
* - refresh dirty_tx ring index (read barrier).
--
2.19.1
^ permalink raw reply related
* Can decnet be deprecated?
From: David Ahern @ 2018-11-25 0:12 UTC (permalink / raw)
To: netdev@vger.kernel.org, David Miller
IPX was moved to staging at the end of last year. Can decnet follow
suit? git log seems to indicate no active development in a very long time.
David
^ 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