Hi everyone,
This is a set of changes for network drivers and core code to get rid of the use of time_t and derived data structures.
I have a longer set of patches that enables me to build kernels with the time_t definition removed completely as a help to find y2038 overflow issues. This is the subset for networking that contains all code that has a reasonable way of fixing at the moment and that is either commonly used (in one of the defconfigs) or that blocks building a whole subsystem.
Most of the patches in this series should be noncontroversial, but the last two that I marked [RFC] are a bit tricky and need input from people that are more familiar with the code than I am. All 12 patches are independent of one another and can be applied in any order, so feel free to pick all that look good.
Patches that are not included here are:
- disabling less common device drivers that I don't have a fix for yet, this includes drivers/net/ethernet/brocade/bna/bfa_ioc.c drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c drivers/net/ethernet/tile/tilegx.c drivers/net/hamradio/baycom_ser_fdx.c drivers/net/wireless/ath/ath10k/core.h drivers/net/wireless/ath/ath9k/ drivers/net/wireless/ath/ath9k/ drivers/net/wireless/atmel.c drivers/net/wireless/prism54/isl_38xx.c drivers/net/wireless/rt2x00/rt2x00debug.c drivers/net/wireless/rtlwifi/ drivers/net/wireless/ti/wlcore/ drivers/staging/ozwpan/ net/atm/mpoa_caches.c net/atm/mpoa_proc.c net/dccp/probe.c net/ipv4/tcp_probe.c net/netfilter/nfnetlink_queue_core.c net/netfilter/nfnetlink_queue_core.c net/netfilter/xt_time.c net/openvswitch/flow.c net/sctp/probe.c net/sunrpc/auth_gss/ net/sunrpc/svcauth_unix.c net/vmw_vsock/af_vsock.c We'll get there eventually, or we an add a dependency to ensure they are not built on 32-bit kernels that need to survive beyond 2038. Most of these should be really easy to fix.
- recvmmsg/sendmmsg system calls: patches have been sent out as part of the syscall series, need a little more work and review
- SIOCGSTAMP/SIOCGSTAMPNS/ ioctl calls: tricky, need to discuss with some folks at kernel summit
- SO_RCVTIMEO/SO_SNDTIMEO/SO_TIMESTAMP/SO_TIMESTAMPNS socket opt: similar and related to the ioctl
- mmapped packet socket: need to create v4 of the API, nontrivial
- pktgen: sends 32-bit timestamps over network, need to find out if using unsigned stamps is good enough
- af_rxpc: similar to pktgen, uses 32-bit times for deadlines
- ppp ioctl: patch is being worked on, nontrivial but doable
Arnd
Arnd Bergmann (12): net: fec: avoid timespec use net: stmmac: avoid using timespec net: igb: avoid using timespec mwifiex: use ktime_get_real for timestamping mwifiex: avoid gettimeofday in ba_threshold setting mac80211: use ktime_get_seconds atm: hide 'struct zatm_t_hist' nfnetlink: use y2038 safe timestamp ipv6: use ktime_t for internal timestamps net: sctp: avoid incorrect time_t use [RFC] ipv4: avoid timespec in timestamp computation [RFC] can: avoid using timeval for uapi
drivers/net/ethernet/freescale/fec_ptp.c | 6 ++-- drivers/net/ethernet/intel/igb/igb.h | 4 +-- drivers/net/ethernet/intel/igb/igb_main.c | 15 +++++----- drivers/net/ethernet/intel/igb/igb_ptp.c | 8 +++--- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 ++++-- drivers/net/wireless/mwifiex/11n_aggr.c | 4 +-- drivers/net/wireless/mwifiex/wmm.c | 15 +++------- include/linux/timekeeping.h | 2 ++ include/uapi/linux/atm_zatm.h | 3 +- include/uapi/linux/can/bcm.h | 7 ++++- kernel/time/timekeeping.c | 34 +++++++++++++++++++++++ net/can/bcm.c | 15 ++++++---- net/ipv4/icmp.c | 8 ++---- net/ipv4/ip_options.c | 9 ++---- net/ipv6/mip6.c | 16 +++++------ net/mac80211/sta_info.c | 8 ++---- net/netfilter/nfnetlink_log.c | 6 ++-- net/sctp/sm_make_chunk.c | 2 +- net/sctp/sm_statefuns.c | 2 +- 19 files changed, 99 insertions(+), 73 deletions(-)
Cc: coreteam@netfilter.org Cc: intel-wired-lan@lists.osuosl.org Cc: linux-api@vger.kernel.org Cc: linux-atm-general@lists.sourceforge.net Cc: linux-can@vger.kernel.org Cc: linux-sctp@vger.kernel.org Cc: linux-wireless@vger.kernel.org Cc: netfilter-devel@vger.kernel.org
The fec_ptp_enable_pps uses an open-coded implementation of ns_to_timespec, which will be removed eventually as it is not y2038-safe on 32-bit architectures. Two more instances of the same code in this file were already converted to use the safe ns_to_timespec64 in commit 6630514fcee ("ptp: fec: use helpers for converting ns to timespec"), this changes the last one as well.
The seconds portion here is actually unused and we could just remove the timespec variable, but using ns_to_timespec64 can still be better as the implementation can be hand-optimized in the future.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Richard Cochran richardcochran@gmail.com Cc: Fugang Duan b38611@freescale.com Cc: Luwei Zhou b45643@freescale.com Cc: Frank Li Frank.Li@freescale.com --- drivers/net/ethernet/freescale/fec_ptp.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c index 1543cf0e8ef6..f9e74461bdc0 100644 --- a/drivers/net/ethernet/freescale/fec_ptp.c +++ b/drivers/net/ethernet/freescale/fec_ptp.c @@ -112,9 +112,8 @@ static int fec_ptp_enable_pps(struct fec_enet_private *fep, uint enable) unsigned long flags; u32 val, tempval; int inc; - struct timespec ts; + struct timespec64 ts; u64 ns; - u32 remainder; val = 0;
if (!(fep->hwts_tx_en || fep->hwts_rx_en)) { @@ -163,8 +162,7 @@ static int fec_ptp_enable_pps(struct fec_enet_private *fep, uint enable) tempval = readl(fep->hwp + FEC_ATIME); /* Convert the ptp local counter to 1588 timestamp */ ns = timecounter_cyc2time(&fep->tc, tempval); - ts.tv_sec = div_u64_rem(ns, 1000000000ULL, &remainder); - ts.tv_nsec = remainder; + ts = ns_to_timespec64(ns);
/* The tempval is less than 3 seconds, and so val is less than * 4 seconds. No overflow for 32bit calculation.
On Wed, Sep 30, 2015 at 01:26:31PM +0200, Arnd Bergmann wrote:
The fec_ptp_enable_pps uses an open-coded implementation of ns_to_timespec, which will be removed eventually as it is not y2038-safe on 32-bit architectures. Two more instances of the same code in this file were already converted to use the safe ns_to_timespec64 in commit 6630514fcee ("ptp: fec: use helpers for converting ns to timespec"), this changes the last one as well.
The seconds portion here is actually unused and we could just remove the timespec variable, but using ns_to_timespec64 can still be better as the implementation can be hand-optimized in the future.
Signed-off-by: Arnd Bergmann arnd@arndb.de
Acked-by: Richard Cochran richardcochran@gmail.com
We want to deprecate the use of 'struct timespec' on 32-bit architectures, as it is will overflow in 2038. The stmmac driver uses it to read the current time, and can simply be changed to use ktime_get_real_ts64() instead.
Because of hardware limitations, there is still an overflow in year 2106, which we cannot really avoid, but this documents the overflow.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Giuseppe Cavallaro peppe.cavallaro@st.com Cc: Richard Cochran richardcochran@gmail.com --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 925f2f8659b8..83a1db1b53f3 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -424,7 +424,7 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr) { struct stmmac_priv *priv = netdev_priv(dev); struct hwtstamp_config config; - struct timespec now; + struct timespec64 now; u64 temp = 0; u32 ptp_v2 = 0; u32 tstamp_all = 0; @@ -621,8 +621,10 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr) priv->default_addend);
/* initialize system time */ - getnstimeofday(&now); - priv->hw->ptp->init_systime(priv->ioaddr, now.tv_sec, + ktime_get_real_ts64(&now); + + /* lower 32 bits of tv_sec are safe until y2106 */ + priv->hw->ptp->init_systime(priv->ioaddr, (u32)now.tv_sec, now.tv_nsec); }
On Wed, Sep 30, 2015 at 01:26:32PM +0200, Arnd Bergmann wrote:
We want to deprecate the use of 'struct timespec' on 32-bit architectures, as it is will overflow in 2038. The stmmac driver uses it to read the current time, and can simply be changed to use ktime_get_real_ts64() instead.
Because of hardware limitations, there is still an overflow in year 2106, which we cannot really avoid, but this documents the overflow.
Signed-off-by: Arnd Bergmann arnd@arndb.de
Acked-by: Richard Cochran richardcochran@gmail.com
We want to deprecate the use of 'struct timespec' on 32-bit architectures, as it is will overflow in 2038. The igb driver uses it to read the current time, and can simply be changed to use ktime_get_real_ts64() instead.
Because of hardware limitations, there is still an overflow in year 2106, which we cannot really avoid, but this documents the overflow.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Jeff Kirsher jeffrey.t.kirsher@intel.com Cc: intel-wired-lan@lists.osuosl.org --- drivers/net/ethernet/intel/igb/igb.h | 4 ++-- drivers/net/ethernet/intel/igb/igb_main.c | 15 ++++++++------- drivers/net/ethernet/intel/igb/igb_ptp.c | 8 ++++---- 3 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h index 212d668dabb3..1a2f1cc44b28 100644 --- a/drivers/net/ethernet/intel/igb/igb.h +++ b/drivers/net/ethernet/intel/igb/igb.h @@ -444,8 +444,8 @@ struct igb_adapter {
struct ptp_pin_desc sdp_config[IGB_N_SDP]; struct { - struct timespec start; - struct timespec period; + struct timespec64 start; + struct timespec64 period; } perout[IGB_N_PEROUT];
char fw_version[32]; diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index e174fbbdba40..911bbadbb994 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -5389,7 +5389,7 @@ static void igb_tsync_interrupt(struct igb_adapter *adapter) { struct e1000_hw *hw = &adapter->hw; struct ptp_clock_event event; - struct timespec ts; + struct timespec64 ts; u32 ack = 0, tsauxc, sec, nsec, tsicr = rd32(E1000_TSICR);
if (tsicr & TSINTR_SYS_WRAP) { @@ -5409,10 +5409,11 @@ static void igb_tsync_interrupt(struct igb_adapter *adapter)
if (tsicr & TSINTR_TT0) { spin_lock(&adapter->tmreg_lock); - ts = timespec_add(adapter->perout[0].start, - adapter->perout[0].period); + ts = timespec64_add(adapter->perout[0].start, + adapter->perout[0].period); + /* u32 conversion of tv_sec is safe until y2106 */ wr32(E1000_TRGTTIML0, ts.tv_nsec); - wr32(E1000_TRGTTIMH0, ts.tv_sec); + wr32(E1000_TRGTTIMH0, (u32)ts.tv_sec); tsauxc = rd32(E1000_TSAUXC); tsauxc |= TSAUXC_EN_TT0; wr32(E1000_TSAUXC, tsauxc); @@ -5423,10 +5424,10 @@ static void igb_tsync_interrupt(struct igb_adapter *adapter)
if (tsicr & TSINTR_TT1) { spin_lock(&adapter->tmreg_lock); - ts = timespec_add(adapter->perout[1].start, - adapter->perout[1].period); + ts = timespec64_add(adapter->perout[1].start, + adapter->perout[1].period); wr32(E1000_TRGTTIML1, ts.tv_nsec); - wr32(E1000_TRGTTIMH1, ts.tv_sec); + wr32(E1000_TRGTTIMH1, (u32)ts.tv_sec); tsauxc = rd32(E1000_TSAUXC); tsauxc |= TSAUXC_EN_TT1; wr32(E1000_TSAUXC, tsauxc); diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c index 5982f28d521a..c44df87c38de 100644 --- a/drivers/net/ethernet/intel/igb/igb_ptp.c +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c @@ -143,7 +143,7 @@ static void igb_ptp_write_i210(struct igb_adapter *adapter, * sub-nanosecond resolution. */ wr32(E1000_SYSTIML, ts->tv_nsec); - wr32(E1000_SYSTIMH, ts->tv_sec); + wr32(E1000_SYSTIMH, (u32)ts->tv_sec); }
/** @@ -479,7 +479,7 @@ static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp, struct e1000_hw *hw = &igb->hw; u32 tsauxc, tsim, tsauxc_mask, tsim_mask, trgttiml, trgttimh, freqout; unsigned long flags; - struct timespec ts; + struct timespec64 ts; int use_freq = 0, pin = -1; s64 ns;
@@ -523,14 +523,14 @@ static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp, } ts.tv_sec = rq->perout.period.sec; ts.tv_nsec = rq->perout.period.nsec; - ns = timespec_to_ns(&ts); + ns = timespec64_to_ns(&ts); ns = ns >> 1; if (on && ns <= 70000000LL) { if (ns < 8LL) return -EINVAL; use_freq = 1; } - ts = ns_to_timespec(ns); + ts = ns_to_timespec64(ns); if (rq->perout.index == 1) { if (use_freq) { tsauxc_mask = TSAUXC_EN_CLK1 | TSAUXC_ST1;
Arnd
As author of these snippets, I went ahead and reviewed. I have one comment, below.
Reviewed-by: Richard Cochran richardcochran@gmail.com
On Wed, Sep 30, 2015 at 01:26:33PM +0200, Arnd Bergmann wrote:
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c index 5982f28d521a..c44df87c38de 100644 --- a/drivers/net/ethernet/intel/igb/igb_ptp.c +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c @@ -143,7 +143,7 @@ static void igb_ptp_write_i210(struct igb_adapter *adapter, * sub-nanosecond resolution. */ wr32(E1000_SYSTIML, ts->tv_nsec);
- wr32(E1000_SYSTIMH, ts->tv_sec);
- wr32(E1000_SYSTIMH, (u32)ts->tv_sec);
This cast is unnecessary, because wr32 is writel, and that parameter is a u32.
Thanks, Richard
} /**
On Thursday 01 October 2015 21:17:45 Richard Cochran wrote:
On Wed, Sep 30, 2015 at 01:26:33PM +0200, Arnd Bergmann wrote:
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c index 5982f28d521a..c44df87c38de 100644 --- a/drivers/net/ethernet/intel/igb/igb_ptp.c +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c @@ -143,7 +143,7 @@ static void igb_ptp_write_i210(struct igb_adapter *adapter, * sub-nanosecond resolution. */ wr32(E1000_SYSTIML, ts->tv_nsec);
wr32(E1000_SYSTIMH, ts->tv_sec);
wr32(E1000_SYSTIMH, (u32)ts->tv_sec);
This cast is unnecessary, because wr32 is writel, and that parameter is a u32.
I tried to use this pattern whenever I convert the 64-bit 'long long' tv_sec member of 'struct timespec64' into a 32-bit number, to annotate the loss of range.
I have thought about defining separate helpers like this
/* result will overflow in 2038 for real time */ static inline s32 time64_to_s32_y2038(time64_t time) { return time; }
/* result will overflow in 2106 for real time */ static inline u32 time64_to_u32_y2106(time64_t time) { return time; }
/* monotonic times can safely be represented as 32 bit */ static inline s32 time64_to_s32_monotonic(time64_t time) { return time; }
This would make it even more explicit, but my fear was that I was adding too much complexity like that.
Arnd
On Thu, Oct 01, 2015 at 10:01:57PM +0200, Arnd Bergmann wrote:
I tried to use this pattern whenever I convert the 64-bit 'long long' tv_sec member of 'struct timespec64' into a 32-bit number, to annotate the loss of range.
Sounds reasonable to me.
I have thought about defining separate helpers like this
...
This would make it even more explicit, but my fear was that I was adding too much complexity like that.
I think a cast plus a comment when needed is clear enough.
Thanks, Richard
The mwifiex_11n_aggregate_pkt() function creates a ktime_t from a timeval returned by do_gettimeofday, which is slow and causes an overflow in 2038 on 32-bit architectures.
This solves both problems by using the appropriate ktime_get_real() function.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Amitkumar Karwar akarwar@marvell.com Cc: Nishant Sarmukadam nishants@marvell.com Cc: Kalle Valo kvalo@codeaurora.org Cc: linux-wireless@vger.kernel.org --- drivers/net/wireless/mwifiex/11n_aggr.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/11n_aggr.c b/drivers/net/wireless/mwifiex/11n_aggr.c index f7c717253a66..78853c51774d 100644 --- a/drivers/net/wireless/mwifiex/11n_aggr.c +++ b/drivers/net/wireless/mwifiex/11n_aggr.c @@ -173,7 +173,6 @@ mwifiex_11n_aggregate_pkt(struct mwifiex_private *priv, int pad = 0, aggr_num = 0, ret; struct mwifiex_tx_param tx_param; struct txpd *ptx_pd = NULL; - struct timeval tv; int headroom = adapter->iface_type == MWIFIEX_USB ? 0 : INTF_HEADER_LEN;
skb_src = skb_peek(&pra_list->skb_head); @@ -203,8 +202,7 @@ mwifiex_11n_aggregate_pkt(struct mwifiex_private *priv, tx_info_aggr->flags |= MWIFIEX_BUF_FLAG_AGGR_PKT; skb_aggr->priority = skb_src->priority;
- do_gettimeofday(&tv); - skb_aggr->tstamp = timeval_to_ktime(tv); + skb_aggr->tstamp = ktime_get_real();
do { /* Check if AMSDU can accommodate this MSDU */
Hi Arnd,
From: Arnd Bergmann [mailto:arnd@arndb.de] Sent: Wednesday, September 30, 2015 4:57 PM To: netdev@vger.kernel.org Cc: y2038@lists.linaro.org; linux-kernel@vger.kernel.org; David S. Miller; Arnd Bergmann; Amitkumar Karwar; Nishant Sarmukadam; Kalle Valo; linux-wireless@vger.kernel.org Subject: [PATCH 04/12] mwifiex: use ktime_get_real for timestamping
The mwifiex_11n_aggregate_pkt() function creates a ktime_t from a timeval returned by do_gettimeofday, which is slow and causes an overflow in 2038 on 32-bit architectures.
This solves both problems by using the appropriate ktime_get_real() function.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Amitkumar Karwar akarwar@marvell.com Cc: Nishant Sarmukadam nishants@marvell.com Cc: Kalle Valo kvalo@codeaurora.org Cc: linux-wireless@vger.kernel.org
drivers/net/wireless/mwifiex/11n_aggr.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/11n_aggr.c b/drivers/net/wireless/mwifiex/11n_aggr.c index f7c717253a66..78853c51774d 100644 --- a/drivers/net/wireless/mwifiex/11n_aggr.c +++ b/drivers/net/wireless/mwifiex/11n_aggr.c @@ -173,7 +173,6 @@ mwifiex_11n_aggregate_pkt(struct mwifiex_private *priv, int pad = 0, aggr_num = 0, ret; struct mwifiex_tx_param tx_param; struct txpd *ptx_pd = NULL;
- struct timeval tv; int headroom = adapter->iface_type == MWIFIEX_USB ? 0 :
INTF_HEADER_LEN;
skb_src = skb_peek(&pra_list->skb_head); @@ -203,8 +202,7 @@ mwifiex_11n_aggregate_pkt(struct mwifiex_private *priv, tx_info_aggr->flags |= MWIFIEX_BUF_FLAG_AGGR_PKT; skb_aggr->priority = skb_src->priority;
- do_gettimeofday(&tv);
- skb_aggr->tstamp = timeval_to_ktime(tv);
skb_aggr->tstamp = ktime_get_real();
do { /* Check if AMSDU can accommodate this MSDU */
-- 2.1.0.rc2
Looks good.
Acked-by: Amitkumar Karwar akarwar@marvell.com
Regards, Amitkumar
mwifiex_get_random_ba_threshold() uses a complex homegrown implementation to generate a pseudo-random number from the current time as returned from do_gettimeofday().
This currently requires two 32-bit divisions plus a couple of other computations that are eventually discarded as only eight bits of the microsecond portion are used at all.
We could replace this with a call to get_random_bytes(), but that might drain the entropy pool too fast if this is called for each packet.
Instead, this patch converts it to use ktime_get_ns(), which is a bit faster than do_gettimeofday(), and then uses a similar algorithm as before, but in a way that takes both the nanosecond and second portion into account for slightly-more-but-still-not-very-random pseudorandom number.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Amitkumar Karwar akarwar@marvell.com Cc: Nishant Sarmukadam nishants@marvell.com Cc: Kalle Valo kvalo@codeaurora.org Cc: linux-wireless@vger.kernel.org --- drivers/net/wireless/mwifiex/wmm.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/wmm.c b/drivers/net/wireless/mwifiex/wmm.c index 173d3663c2e0..878d358063dc 100644 --- a/drivers/net/wireless/mwifiex/wmm.c +++ b/drivers/net/wireless/mwifiex/wmm.c @@ -117,22 +117,15 @@ mwifiex_wmm_allocate_ralist_node(struct mwifiex_adapter *adapter, const u8 *ra) */ static u8 mwifiex_get_random_ba_threshold(void) { - u32 sec, usec; - struct timeval ba_tstamp; - u8 ba_threshold; - + u64 ns; /* setup ba_packet_threshold here random number between * [BA_SETUP_PACKET_OFFSET, * BA_SETUP_PACKET_OFFSET+BA_SETUP_MAX_PACKET_THRESHOLD-1] */ + ns = ktime_get_ns(); + ns += (ns >> 32) + (ns >> 16);
- do_gettimeofday(&ba_tstamp); - sec = (ba_tstamp.tv_sec & 0xFFFF) + (ba_tstamp.tv_sec >> 16); - usec = (ba_tstamp.tv_usec & 0xFFFF) + (ba_tstamp.tv_usec >> 16); - ba_threshold = (((sec << 16) + usec) % BA_SETUP_MAX_PACKET_THRESHOLD) - + BA_SETUP_PACKET_OFFSET; - - return ba_threshold; + return ((u8)ns % BA_SETUP_MAX_PACKET_THRESHOLD) + BA_SETUP_PACKET_OFFSET; }
/*
Hi Arnd,
From: Arnd Bergmann [mailto:arnd@arndb.de] Sent: Wednesday, September 30, 2015 4:57 PM To: netdev@vger.kernel.org Cc: y2038@lists.linaro.org; linux-kernel@vger.kernel.org; David S. Miller; Arnd Bergmann; Amitkumar Karwar; Nishant Sarmukadam; Kalle Valo; linux-wireless@vger.kernel.org Subject: [PATCH 05/12] mwifiex: avoid gettimeofday in ba_threshold setting
mwifiex_get_random_ba_threshold() uses a complex homegrown implementation to generate a pseudo-random number from the current time as returned from do_gettimeofday().
This currently requires two 32-bit divisions plus a couple of other computations that are eventually discarded as only eight bits of the microsecond portion are used at all.
We could replace this with a call to get_random_bytes(), but that might drain the entropy pool too fast if this is called for each packet.
Instead, this patch converts it to use ktime_get_ns(), which is a bit faster than do_gettimeofday(), and then uses a similar algorithm as before, but in a way that takes both the nanosecond and second portion into account for slightly-more-but-still-not-very-random pseudorandom number.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Amitkumar Karwar akarwar@marvell.com Cc: Nishant Sarmukadam nishants@marvell.com Cc: Kalle Valo kvalo@codeaurora.org Cc: linux-wireless@vger.kernel.org
drivers/net/wireless/mwifiex/wmm.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/wmm.c b/drivers/net/wireless/mwifiex/wmm.c index 173d3663c2e0..878d358063dc 100644 --- a/drivers/net/wireless/mwifiex/wmm.c +++ b/drivers/net/wireless/mwifiex/wmm.c @@ -117,22 +117,15 @@ mwifiex_wmm_allocate_ralist_node(struct mwifiex_adapter *adapter, const u8 *ra) */ static u8 mwifiex_get_random_ba_threshold(void) {
- u32 sec, usec;
- struct timeval ba_tstamp;
- u8 ba_threshold;
- u64 ns; /* setup ba_packet_threshold here random number between
*/
- [BA_SETUP_PACKET_OFFSET,
- BA_SETUP_PACKET_OFFSET+BA_SETUP_MAX_PACKET_THRESHOLD-1]
- ns = ktime_get_ns();
- ns += (ns >> 32) + (ns >> 16);
- do_gettimeofday(&ba_tstamp);
- sec = (ba_tstamp.tv_sec & 0xFFFF) + (ba_tstamp.tv_sec >> 16);
- usec = (ba_tstamp.tv_usec & 0xFFFF) + (ba_tstamp.tv_usec >> 16);
- ba_threshold = (((sec << 16) + usec) %
BA_SETUP_MAX_PACKET_THRESHOLD)
+ BA_SETUP_PACKET_OFFSET;
- return ba_threshold;
- return ((u8)ns % BA_SETUP_MAX_PACKET_THRESHOLD) +
+BA_SETUP_PACKET_OFFSET; }
/*
Looks fine to me. Acked-by: Amitkumar Karwar akarwar@marvell.com
Regards, Amitkumar
The mac80211 code uses ktime_get_ts to measure the connected time. As this uses monotonic time, it is y2038 safe on 32-bit systems, but we still want to deprecate the use of 'timespec' because most other users are broken.
This changes the code to use ktime_get_seconds() instead, which avoids the timespec structure and is slightly more efficient.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Johannes Berg johannes@sipsolutions.net Cc: linux-wireless@vger.kernel.org --- net/mac80211/sta_info.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c index 64f1936350c6..c3644458e2ee 100644 --- a/net/mac80211/sta_info.c +++ b/net/mac80211/sta_info.c @@ -303,7 +303,6 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata, struct ieee80211_local *local = sdata->local; struct ieee80211_hw *hw = &local->hw; struct sta_info *sta; - struct timespec uptime; int i;
sta = kzalloc(sizeof(*sta) + hw->sta_data_size, gfp); @@ -339,8 +338,7 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata, /* Mark TID as unreserved */ sta->reserved_tid = IEEE80211_TID_UNRESERVED;
- ktime_get_ts(&uptime); - sta->last_connected = uptime.tv_sec; + sta->last_connected = ktime_get_seconds(); ewma_signal_init(&sta->avg_signal); for (i = 0; i < ARRAY_SIZE(sta->chain_signal_avg); i++) ewma_signal_init(&sta->chain_signal_avg[i]); @@ -1813,7 +1811,6 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo) struct ieee80211_sub_if_data *sdata = sta->sdata; struct ieee80211_local *local = sdata->local; struct rate_control_ref *ref = NULL; - struct timespec uptime; u32 thr = 0; int i, ac;
@@ -1838,8 +1835,7 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo) BIT(NL80211_STA_INFO_RX_DROP_MISC) | BIT(NL80211_STA_INFO_BEACON_LOSS);
- ktime_get_ts(&uptime); - sinfo->connected_time = uptime.tv_sec - sta->last_connected; + sinfo->connected_time = ktime_get_seconds() - sta->last_connected; sinfo->inactive_time = jiffies_to_msecs(jiffies - sta->last_rx);
if (!(sinfo->filled & (BIT(NL80211_STA_INFO_TX_BYTES64) |
The zatm_t_hist structure is not used anywhere in the kernel, but is exported to user space. As we are trying to eliminate uses of time_t in the kernel for y2038 compatibility, the current definition triggers checking tools because it contains 'struct timeval'.
We can work around this by adding '#ifdef __KERNEL__'. I could not find out what the structure is actually used for, so this is the safe choice in case there is some user space tool that relies on the definition.
If we are sure that nothing in user space relies on the structure, we can instead remove the definition completely.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Chas Williams 3chas3@gmail.com Cc: linux-atm-general@lists.sourceforge.net --- include/uapi/linux/atm_zatm.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/atm_zatm.h b/include/uapi/linux/atm_zatm.h index 10f0fa29454f..2908ea86e6f2 100644 --- a/include/uapi/linux/atm_zatm.h +++ b/include/uapi/linux/atm_zatm.h @@ -35,11 +35,12 @@ struct zatm_pool_req { struct zatm_pool_info info; /* actual information */ };
+#ifndef __KERNEL__ struct zatm_t_hist { struct timeval real; /* real (wall-clock) time */ struct timeval expected; /* expected real time */ }; - +#endif
#define ZATM_OAM_POOL 0 /* free buffer pool for OAM cells */ #define ZATM_AAL0_POOL 1 /* free buffer pool for AAL0 cells */
On Wed, 2015-09-30 at 13:26 +0200, Arnd Bergmann wrote:
The zatm_t_hist structure is not used anywhere in the kernel, but is exported to user space. As we are trying to eliminate uses of time_t in the kernel for y2038 compatibility, the current definition triggers checking tools because it contains 'struct timeval'.
We can work around this by adding '#ifdef __KERNEL__'. I could not find out what the structure is actually used for, so this is the safe choice in case there is some user space tool that relies on the definition.
If we are sure that nothing in user space relies on the structure, we can instead remove the definition completely.
It was used by the ZATM_GETHIST ioctl which is long since gone in the kernel driver. You can just remove this.
On Wednesday 30 September 2015 11:24:35 Charles Williams wrote:
On Wed, 2015-09-30 at 13:26 +0200, Arnd Bergmann wrote:
The zatm_t_hist structure is not used anywhere in the kernel, but is exported to user space. As we are trying to eliminate uses of time_t in the kernel for y2038 compatibility, the current definition triggers checking tools because it contains 'struct timeval'.
We can work around this by adding '#ifdef __KERNEL__'. I could not find out what the structure is actually used for, so this is the safe choice in case there is some user space tool that relies on the definition.
If we are sure that nothing in user space relies on the structure, we can instead remove the definition completely.
It was used by the ZATM_GETHIST ioctl which is long since gone in the kernel driver. You can just remove this.
Perfect, I've updated the patch accordingly, see follow-up mail
Thanks,
Arnd
The zatm_t_hist structure is not used anywhere in the kernel, but is exported to user space. As we are trying to eliminate uses of time_t in the kernel for y2038 compatibility, the current definition triggers checking tools because it contains 'struct timeval'.
As pointed out by Chas Williams, the only user of this structure was the ZATM_GETHIST ioctl command that has been removed a long time ago, and we can remove the structure as well without breaking any user space.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Chas Williams 3chas3@gmail.com Cc: linux-atm-general@lists.sourceforge.net
diff --git a/include/uapi/linux/atm_zatm.h b/include/uapi/linux/atm_zatm.h index 10f0fa29454f..9c9c6ad55f14 100644 --- a/include/uapi/linux/atm_zatm.h +++ b/include/uapi/linux/atm_zatm.h @@ -35,12 +35,6 @@ struct zatm_pool_req { struct zatm_pool_info info; /* actual information */ };
-struct zatm_t_hist { - struct timeval real; /* real (wall-clock) time */ - struct timeval expected; /* expected real time */ -}; - - #define ZATM_OAM_POOL 0 /* free buffer pool for OAM cells */ #define ZATM_AAL0_POOL 1 /* free buffer pool for AAL0 cells */ #define ZATM_AAL5_POOL_BASE 2 /* first AAL5 free buffer pool */
The __build_packet_message function fills a nfulnl_msg_packet_timestamp structure that uses 64-bit seconds and is therefore y2038 safe, but it uses an intermediate 'struct timespec' which is not.
This trivially changes the code to use 'struct timespec64' instead, to correct the result on 32-bit architectures.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Pablo Neira Ayuso pablo@netfilter.org Cc: Patrick McHardy kaber@trash.net Cc: Jozsef Kadlecsik kadlec@blackhole.kfki.hu Cc: netfilter-devel@vger.kernel.org Cc: coreteam@netfilter.org --- net/netfilter/nfnetlink_log.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c index 4670821b569d..cc2300f4e177 100644 --- a/net/netfilter/nfnetlink_log.c +++ b/net/netfilter/nfnetlink_log.c @@ -538,9 +538,9 @@ __build_packet_message(struct nfnl_log_net *log,
if (skb->tstamp.tv64) { struct nfulnl_msg_packet_timestamp ts; - struct timeval tv = ktime_to_timeval(skb->tstamp); - ts.sec = cpu_to_be64(tv.tv_sec); - ts.usec = cpu_to_be64(tv.tv_usec); + struct timespec64 kts = ktime_to_timespec64(skb->tstamp); + ts.sec = cpu_to_be64(kts.tv_sec); + ts.usec = cpu_to_be64(kts.tv_nsec / NSEC_PER_USEC);
if (nla_put(inst->skb, NFULA_TIMESTAMP, sizeof(ts), &ts)) goto nla_put_failure;
On Wed, Sep 30, 2015 at 01:26:38PM +0200, Arnd Bergmann wrote:
The __build_packet_message function fills a nfulnl_msg_packet_timestamp structure that uses 64-bit seconds and is therefore y2038 safe, but it uses an intermediate 'struct timespec' which is not.
This trivially changes the code to use 'struct timespec64' instead, to correct the result on 32-bit architectures.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Pablo Neira Ayuso pablo@netfilter.org Cc: Patrick McHardy kaber@trash.net Cc: Jozsef Kadlecsik kadlec@blackhole.kfki.hu Cc: netfilter-devel@vger.kernel.org Cc: coreteam@netfilter.org
Acked-by: Pablo Neira Ayuso pablo@netfilter.org
BTW, I don't see the patch for nfnetlink_queue and I think I have seen it in the diffstat from your cover letter.
On Friday 02 October 2015 14:53:55 Pablo Neira Ayuso wrote:
On Wed, Sep 30, 2015 at 01:26:38PM +0200, Arnd Bergmann wrote:
The __build_packet_message function fills a nfulnl_msg_packet_timestamp structure that uses 64-bit seconds and is therefore y2038 safe, but it uses an intermediate 'struct timespec' which is not.
This trivially changes the code to use 'struct timespec64' instead, to correct the result on 32-bit architectures.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Pablo Neira Ayuso pablo@netfilter.org Cc: Patrick McHardy kaber@trash.net Cc: Jozsef Kadlecsik kadlec@blackhole.kfki.hu Cc: netfilter-devel@vger.kernel.org Cc: coreteam@netfilter.org
Acked-by: Pablo Neira Ayuso pablo@netfilter.org
Thanks
BTW, I don't see the patch for nfnetlink_queue and I think I have seen it in the diffstat from your cover letter.
My text must have been unclear. What I meant is that I have identified that file as needing a patch (among 120 other files) but have not written one.
This one is trivial, it just needs replacing this code
if (entskb->tstamp.tv64) { struct nfqnl_msg_packet_timestamp ts; struct timeval tv = ktime_to_timeval(entskb->tstamp); ts.sec = cpu_to_be64(tv.tv_sec); ts.usec = cpu_to_be64(tv.tv_usec);
if (nla_put(skb, NFQA_TIMESTAMP, sizeof(ts), &ts)) goto nla_put_failure; }
with a version using ktime_to_timespec64. If you or someone else does this, that makes one less file for me to track.
Arnd
The ipv6 mip6 implementation is one of only a few users of the skb_get_timestamp() function in the kernel, which is both unsafe on 32-bit architectures because of the 2038 overflow, and slightly less efficient than the skb_get_ktime() based approach.
This converts the function call and the mip6_report_rate_limiter structure that stores the time stamp, eliminating all uses of timeval in the ipv6 code.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Alexey Kuznetsov kuznet@ms2.inr.ac.ru Cc: James Morris jmorris@namei.org Cc: Hideaki YOSHIFUJI yoshfuji@linux-ipv6.org Cc: Patrick McHardy kaber@trash.net --- net/ipv6/mip6.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/net/ipv6/mip6.c b/net/ipv6/mip6.c index b9779d441b12..60c79a08e14a 100644 --- a/net/ipv6/mip6.c +++ b/net/ipv6/mip6.c @@ -118,7 +118,7 @@ static int mip6_mh_filter(struct sock *sk, struct sk_buff *skb)
struct mip6_report_rate_limiter { spinlock_t lock; - struct timeval stamp; + ktime_t stamp; int iif; struct in6_addr src; struct in6_addr dst; @@ -184,20 +184,18 @@ static int mip6_destopt_output(struct xfrm_state *x, struct sk_buff *skb) return 0; }
-static inline int mip6_report_rl_allow(struct timeval *stamp, +static inline int mip6_report_rl_allow(ktime_t stamp, const struct in6_addr *dst, const struct in6_addr *src, int iif) { int allow = 0;
spin_lock_bh(&mip6_report_rl.lock); - if (mip6_report_rl.stamp.tv_sec != stamp->tv_sec || - mip6_report_rl.stamp.tv_usec != stamp->tv_usec || + if (!ktime_equal(mip6_report_rl.stamp, stamp) || mip6_report_rl.iif != iif || !ipv6_addr_equal(&mip6_report_rl.src, src) || !ipv6_addr_equal(&mip6_report_rl.dst, dst)) { - mip6_report_rl.stamp.tv_sec = stamp->tv_sec; - mip6_report_rl.stamp.tv_usec = stamp->tv_usec; + mip6_report_rl.stamp = stamp; mip6_report_rl.iif = iif; mip6_report_rl.src = *src; mip6_report_rl.dst = *dst; @@ -216,7 +214,7 @@ static int mip6_destopt_reject(struct xfrm_state *x, struct sk_buff *skb, struct ipv6_destopt_hao *hao = NULL; struct xfrm_selector sel; int offset; - struct timeval stamp; + ktime_t stamp; int err = 0;
if (unlikely(fl6->flowi6_proto == IPPROTO_MH && @@ -230,9 +228,9 @@ static int mip6_destopt_reject(struct xfrm_state *x, struct sk_buff *skb, (skb_network_header(skb) + offset); }
- skb_get_timestamp(skb, &stamp); + stamp = skb_get_ktime(skb);
- if (!mip6_report_rl_allow(&stamp, &ipv6_hdr(skb)->daddr, + if (!mip6_report_rl_allow(stamp, &ipv6_hdr(skb)->daddr, hao ? &hao->addr : &ipv6_hdr(skb)->saddr, opt->iif)) goto out;
We want to avoid using time_t in the kernel because of the y2038 overflow problem. The use in sctp is not for storing seconds at all, but instead uses microseconds and is passed as 32-bit on all machines.
This patch changes the type to u32, which better fits the use.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Vlad Yasevich vyasevich@gmail.com Cc: Neil Horman nhorman@tuxdriver.com Cc: linux-sctp@vger.kernel.org --- net/sctp/sm_make_chunk.c | 2 +- net/sctp/sm_statefuns.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c index 7954c52e1794..763e06a55155 100644 --- a/net/sctp/sm_make_chunk.c +++ b/net/sctp/sm_make_chunk.c @@ -2494,7 +2494,7 @@ static int sctp_process_param(struct sctp_association *asoc, __u16 sat; int retval = 1; sctp_scope_t scope; - time_t stale; + u32 stale; struct sctp_af *af; union sctp_addr_param *addr_param; struct sctp_transport *t; diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index d7eaa7354cf7..6f46aa16cb76 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -2306,7 +2306,7 @@ static sctp_disposition_t sctp_sf_do_5_2_6_stale(struct net *net, sctp_cmd_seq_t *commands) { struct sctp_chunk *chunk = arg; - time_t stale; + u32 stale; sctp_cookie_preserve_param_t bht; sctp_errhdr_t *err; struct sctp_chunk *reply;
On Wed, Sep 30, 2015 at 01:26:40PM +0200, Arnd Bergmann wrote:
We want to avoid using time_t in the kernel because of the y2038 overflow problem. The use in sctp is not for storing seconds at all, but instead uses microseconds and is passed as 32-bit on all machines.
This patch changes the type to u32, which better fits the use.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Vlad Yasevich vyasevich@gmail.com Cc: Neil Horman nhorman@tuxdriver.com Cc: linux-sctp@vger.kernel.org
net/sctp/sm_make_chunk.c | 2 +- net/sctp/sm_statefuns.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c index 7954c52e1794..763e06a55155 100644 --- a/net/sctp/sm_make_chunk.c +++ b/net/sctp/sm_make_chunk.c @@ -2494,7 +2494,7 @@ static int sctp_process_param(struct sctp_association *asoc, __u16 sat; int retval = 1; sctp_scope_t scope;
- time_t stale;
- u32 stale; struct sctp_af *af; union sctp_addr_param *addr_param; struct sctp_transport *t;
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index d7eaa7354cf7..6f46aa16cb76 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -2306,7 +2306,7 @@ static sctp_disposition_t sctp_sf_do_5_2_6_stale(struct net *net, sctp_cmd_seq_t *commands) { struct sctp_chunk *chunk = arg;
- time_t stale;
- u32 stale; sctp_cookie_preserve_param_t bht; sctp_errhdr_t *err; struct sctp_chunk *reply;
-- 2.1.0.rc2
Assignments to this variable use ntohl, won't this change risk overflow?
Neil
On Wed, Sep 30, 2015 at 09:57:31AM -0400, Neil Horman wrote:
On Wed, Sep 30, 2015 at 01:26:40PM +0200, Arnd Bergmann wrote:
We want to avoid using time_t in the kernel because of the y2038 overflow problem. The use in sctp is not for storing seconds at all, but instead uses microseconds and is passed as 32-bit on all machines.
This patch changes the type to u32, which better fits the use.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Vlad Yasevich vyasevich@gmail.com Cc: Neil Horman nhorman@tuxdriver.com Cc: linux-sctp@vger.kernel.org
net/sctp/sm_make_chunk.c | 2 +- net/sctp/sm_statefuns.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c index 7954c52e1794..763e06a55155 100644 --- a/net/sctp/sm_make_chunk.c +++ b/net/sctp/sm_make_chunk.c @@ -2494,7 +2494,7 @@ static int sctp_process_param(struct sctp_association *asoc, __u16 sat; int retval = 1; sctp_scope_t scope;
- time_t stale;
- u32 stale; struct sctp_af *af; union sctp_addr_param *addr_param; struct sctp_transport *t;
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index d7eaa7354cf7..6f46aa16cb76 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -2306,7 +2306,7 @@ static sctp_disposition_t sctp_sf_do_5_2_6_stale(struct net *net, sctp_cmd_seq_t *commands) { struct sctp_chunk *chunk = arg;
- time_t stale;
- u32 stale; sctp_cookie_preserve_param_t bht; sctp_errhdr_t *err; struct sctp_chunk *reply;
-- 2.1.0.rc2
Assignments to this variable use ntohl, won't this change risk overflow?
Neil
But isn't ntohl always 4 bytes long?
Marcelo
On Wed, Sep 30, 2015 at 11:15:20AM -0300, Marcelo Ricardo Leitner wrote:
On Wed, Sep 30, 2015 at 09:57:31AM -0400, Neil Horman wrote:
On Wed, Sep 30, 2015 at 01:26:40PM +0200, Arnd Bergmann wrote:
We want to avoid using time_t in the kernel because of the y2038 overflow problem. The use in sctp is not for storing seconds at all, but instead uses microseconds and is passed as 32-bit on all machines.
This patch changes the type to u32, which better fits the use.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Vlad Yasevich vyasevich@gmail.com Cc: Neil Horman nhorman@tuxdriver.com Cc: linux-sctp@vger.kernel.org
net/sctp/sm_make_chunk.c | 2 +- net/sctp/sm_statefuns.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c index 7954c52e1794..763e06a55155 100644 --- a/net/sctp/sm_make_chunk.c +++ b/net/sctp/sm_make_chunk.c @@ -2494,7 +2494,7 @@ static int sctp_process_param(struct sctp_association *asoc, __u16 sat; int retval = 1; sctp_scope_t scope;
- time_t stale;
- u32 stale; struct sctp_af *af; union sctp_addr_param *addr_param; struct sctp_transport *t;
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index d7eaa7354cf7..6f46aa16cb76 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -2306,7 +2306,7 @@ static sctp_disposition_t sctp_sf_do_5_2_6_stale(struct net *net, sctp_cmd_seq_t *commands) { struct sctp_chunk *chunk = arg;
- time_t stale;
- u32 stale; sctp_cookie_preserve_param_t bht; sctp_errhdr_t *err; struct sctp_chunk *reply;
-- 2.1.0.rc2
Assignments to this variable use ntohl, won't this change risk overflow?
Neil
But isn't ntohl always 4 bytes long?
Oh, you might be right. I was thinking it was 8, but I'm wrong
Acked-by: Neil Horman nhorman@tuxdriver.com
Marcelo
-- To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 30, 2015 at 01:26:40PM +0200, Arnd Bergmann wrote:
We want to avoid using time_t in the kernel because of the y2038 overflow problem. The use in sctp is not for storing seconds at all, but instead uses microseconds and is passed as 32-bit on all machines.
This patch changes the type to u32, which better fits the use.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Vlad Yasevich vyasevich@gmail.com Cc: Neil Horman nhorman@tuxdriver.com Cc: linux-sctp@vger.kernel.org
Acked-by: Marcelo Ricardo Leitner marcelo.leitner@gmail.com
net/sctp/sm_make_chunk.c | 2 +- net/sctp/sm_statefuns.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c index 7954c52e1794..763e06a55155 100644 --- a/net/sctp/sm_make_chunk.c +++ b/net/sctp/sm_make_chunk.c @@ -2494,7 +2494,7 @@ static int sctp_process_param(struct sctp_association *asoc, __u16 sat; int retval = 1; sctp_scope_t scope;
- time_t stale;
- u32 stale; struct sctp_af *af; union sctp_addr_param *addr_param; struct sctp_transport *t;
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index d7eaa7354cf7..6f46aa16cb76 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -2306,7 +2306,7 @@ static sctp_disposition_t sctp_sf_do_5_2_6_stale(struct net *net, sctp_cmd_seq_t *commands) { struct sctp_chunk *chunk = arg;
- time_t stale;
- u32 stale; sctp_cookie_preserve_param_t bht; sctp_errhdr_t *err; struct sctp_chunk *reply;
-- 2.1.0.rc2
-- To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
This is an attempt to avoid the use of timespec in ipv4, where getnstimeofday() used to be used for computing the number of milliseconds since midnight, in three places.
That computation would overflow in 2038 on 32-bit machines, and the normal workaround for this is to use timespec64, which in turn requires an expensive div_s64_mod() function call for calculating the seconds modulo 86400.
Instead, this approach introduces a new generic helper function that does this more efficiently, by using only a 32-bit modulo (which the compiler can turn into two multiplications), relying on 39 bits to be sufficient for the current time of day. This is roughly 100 times faster than a full divmod operation on ARM.
As a further optimization, this does not use the exact nanosecond value but instead relies tk_xtime() to report the time of the last jiffy, which is slightly less accurate, depending on the value of HZ.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Alexey Kuznetsov kuznet@ms2.inr.ac.ru Cc: James Morris jmorris@namei.org Cc: Hideaki YOSHIFUJI yoshfuji@linux-ipv6.org Cc: Patrick McHardy kaber@trash.net Cc: John Stultz john.stultz@linaro.org Cc: Thomas Gleixner tglx@linutronix.de --- include/linux/timekeeping.h | 2 ++ kernel/time/timekeeping.c | 34 ++++++++++++++++++++++++++++++++++ net/ipv4/icmp.c | 8 +++----- net/ipv4/ip_options.c | 9 ++------- 4 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h index ca2eaa9077eb..3e126f8c2876 100644 --- a/include/linux/timekeeping.h +++ b/include/linux/timekeeping.h @@ -44,6 +44,8 @@ extern void ktime_get_ts64(struct timespec64 *ts); extern time64_t ktime_get_seconds(void); extern time64_t ktime_get_real_seconds(void);
+extern u32 ktime_get_ms_since_midnight(void); + extern int __getnstimeofday64(struct timespec64 *tv); extern void getnstimeofday64(struct timespec64 *tv); extern void getboottime64(struct timespec64 *ts); diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index ed5049ff94c5..3a1e030fa969 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -846,6 +846,40 @@ time64_t ktime_get_real_seconds(void) } EXPORT_SYMBOL_GPL(ktime_get_real_seconds);
+#if IS_ENABLED(CONFIG_INET) +u32 ktime_get_ms_of_day(void) +{ + struct timekeeper *tk = &tk_core.timekeeper; + struct timespec64 now; + unsigned long seq; + u32 ms; + + /* we assume that the coarse time is good enough here */ + do { + seq = read_seqcount_begin(&tk_core.seq); + + now = tk_xtime(tk); + } while (read_seqcount_retry(&tk_core.seq, seq)); + + /* + * efficiently calculate the milliseconds since midnight: + * 86400 seconds per day == 2^7 * 675, which helps us + * replace an expensive div_s64_rem() with a hand-written + * 39-bit modulo on 32-bit architectures. + */ + if (!IS_ENABLED(CONFIG_64BIT)) + ms = (now.tv_sec & 0x7f) * MSEC_PER_SEC + + ((u32)(now.tv_sec >> 7) % 675) * 0x80 * MSEC_PER_SEC; + else + ms = (now.tv_sec % 86400) * MSEC_PER_SEC; + + ms += now.tv_nsec / NSEC_PER_MSEC; + + return ms; +} +EXPORT_SYMBOL_GPL(ktime_get_ms_since_midnight); +#endif + #ifdef CONFIG_NTP_PPS
/** diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c index e5eb8ac4089d..b1c53f2f7bd5 100644 --- a/net/ipv4/icmp.c +++ b/net/ipv4/icmp.c @@ -914,7 +914,6 @@ static bool icmp_echo(struct sk_buff *skb) */ static bool icmp_timestamp(struct sk_buff *skb) { - struct timespec tv; struct icmp_bxm icmp_param; /* * Too short. @@ -923,11 +922,10 @@ static bool icmp_timestamp(struct sk_buff *skb) goto out_err;
/* - * Fill in the current time as ms since midnight UT: + * Fill in the current time as ms since midnight UT, + * this could probably be done faster. */ - getnstimeofday(&tv); - icmp_param.data.times[1] = htonl((tv.tv_sec % 86400) * MSEC_PER_SEC + - tv.tv_nsec / NSEC_PER_MSEC); + icmp_param.data.times[1] = htonl(ktime_get_ms_since_midnight()); icmp_param.data.times[2] = icmp_param.data.times[1]; if (skb_copy_bits(skb, 0, &icmp_param.data.times[0], 4)) BUG(); diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c index bd246792360b..339ce528ecae 100644 --- a/net/ipv4/ip_options.c +++ b/net/ipv4/ip_options.c @@ -58,10 +58,8 @@ void ip_options_build(struct sk_buff *skb, struct ip_options *opt, if (opt->ts_needaddr) ip_rt_get_source(iph+opt->ts+iph[opt->ts+2]-9, skb, rt); if (opt->ts_needtime) { - struct timespec tv; __be32 midtime; - getnstimeofday(&tv); - midtime = htonl((tv.tv_sec % 86400) * MSEC_PER_SEC + tv.tv_nsec / NSEC_PER_MSEC); + midtime = htonl(ktime_get_ms_since_midnight()); memcpy(iph+opt->ts+iph[opt->ts+2]-5, &midtime, 4); } return; @@ -415,10 +413,7 @@ int ip_options_compile(struct net *net, break; } if (timeptr) { - struct timespec tv; - u32 midtime; - getnstimeofday(&tv); - midtime = (tv.tv_sec % 86400) * MSEC_PER_SEC + tv.tv_nsec / NSEC_PER_MSEC; + u32 midtime = ktime_get_ms_since_midnight(); put_unaligned_be32(midtime, timeptr); opt->is_changed = 1; }
Hi Arnd,
[auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
config: mn10300-asb2364_defconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/ma... -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout e1358094d427405462a47d6d2650458b689e55d9 # save the attached .config to linux build tree make.cross ARCH=mn10300
All error/warnings (new ones prefixed by >>):
am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. kernel/built-in.o: In function `current_thread_info': (___ksymtab_gpl+ktime_get_ms_since_midnight+0x0): undefined reference to `ktime_get_ms_since_midnight' am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. net/built-in.o: In function `register_netevent_notifier':
net/core/netevent.c:29: undefined reference to `ktime_get_ms_since_midnight' net/core/netevent.c:29: undefined reference to `ktime_get_ms_since_midnight'
net/core/netevent.c:17: undefined reference to `ktime_get_ms_since_midnight'
vim +29 net/core/netevent.c
792d1932 Tom Tucker 2006-07-30 23 /** 792d1932 Tom Tucker 2006-07-30 24 * register_netevent_notifier - register a netevent notifier block 792d1932 Tom Tucker 2006-07-30 25 * @nb: notifier 792d1932 Tom Tucker 2006-07-30 26 * 792d1932 Tom Tucker 2006-07-30 27 * Register a notifier to be called when a netevent occurs. 792d1932 Tom Tucker 2006-07-30 28 * The notifier passed is linked into the kernel structures and must 792d1932 Tom Tucker 2006-07-30 @29 * not be reused until it has been unregistered. A negative errno code 792d1932 Tom Tucker 2006-07-30 30 * is returned on a failure. 792d1932 Tom Tucker 2006-07-30 31 */ 792d1932 Tom Tucker 2006-07-30 32 int register_netevent_notifier(struct notifier_block *nb)
:::::: The code at line 29 was first introduced by commit :::::: 792d1932e319ff8ba01361e7d151b1794c55c31f [NET]: Network Event Notifier Mechanism.
:::::: TO: Tom Tucker tom@opengridcomputing.com :::::: CC: David S. Miller davem@sunset.davemloft.net
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Wednesday 30 September 2015 19:55:43 kbuild test robot wrote:
Hi Arnd,
[auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
config: mn10300-asb2364_defconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/ma... -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout e1358094d427405462a47d6d2650458b689e55d9 # save the attached .config to linux build tree make.cross ARCH=mn10300
All error/warnings (new ones prefixed by >>):
am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. kernel/built-in.o: In function `current_thread_info': (___ksymtab_gpl+ktime_get_ms_since_midnight+0x0): undefined reference to `ktime_get_ms_since_midnight' am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. am33_2.0-linux-ld: Dwarf Error: mangled line number section. net/built-in.o: In function `register_netevent_notifier':
net/core/netevent.c:29: undefined reference to `ktime_get_ms_since_midnight' net/core/netevent.c:29: undefined reference to `ktime_get_ms_since_midnight'
net/core/netevent.c:17: undefined reference to `ktime_get_ms_since_midnight'
Thanks for the helpful report, I love the new feature of grabbing the patches from the list.
Moreover, sorry for screwing up here, I changed the function name before sending it out and only compiled the files individually but did not notice that I only changed it in three out of four places because I did not try to relink the kernel.
I'll send an updated version.
Arnd
This is an attempt to avoid the use of timespec in ipv4, where getnstimeofday() used to be used for computing the number of milliseconds since midnight, in three places.
That computation would overflow in 2038 on 32-bit machines, and the normal workaround for this is to use timespec64, which in turn requires an expensive div_s64_mod() function call for calculating the seconds modulo 86400.
Instead, this approach introduces a new generic helper function that does this more efficiently, by using only a 32-bit modulo (which the compiler can turn into two multiplications), relying on 39 bits to be sufficient for the current time of day. This is roughly 100 times faster than a full divmod operation on ARM.
As a further optimization, this does not use the exact nanosecond value but instead relies tk_xtime() to report the time of the last jiffy, which is slightly less accurate, depending on the value of HZ.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Alexey Kuznetsov kuznet@ms2.inr.ac.ru Cc: James Morris jmorris@namei.org Cc: Hideaki YOSHIFUJI yoshfuji@linux-ipv6.org Cc: Patrick McHardy kaber@trash.net Cc: John Stultz john.stultz@linaro.org Cc: Thomas Gleixner tglx@linutronix.de --- v2: fixed build error reported by lkp@intel.com
diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h index ec89d846324c..db8fc5171294 100644 --- a/include/linux/timekeeping.h +++ b/include/linux/timekeeping.h @@ -38,6 +38,8 @@ extern void ktime_get_ts64(struct timespec64 *ts); extern time64_t ktime_get_seconds(void); extern time64_t ktime_get_real_seconds(void);
+extern u32 ktime_get_ms_since_midnight(void); + extern int __getnstimeofday64(struct timespec64 *tv); extern void getnstimeofday64(struct timespec64 *tv); extern void getboottime64(struct timespec64 *ts); diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 274ed5e88456..44ecd7058946 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -846,6 +846,40 @@ time64_t ktime_get_real_seconds(void) } EXPORT_SYMBOL_GPL(ktime_get_real_seconds);
+#if IS_ENABLED(CONFIG_INET) +u32 ktime_get_ms_since_midnight(void) +{ + struct timekeeper *tk = &tk_core.timekeeper; + struct timespec64 now; + unsigned long seq; + u32 ms; + + /* we assume that the coarse time is good enough here */ + do { + seq = read_seqcount_begin(&tk_core.seq); + + now = tk_xtime(tk); + } while (read_seqcount_retry(&tk_core.seq, seq)); + + /* + * efficiently calculate the milliseconds since midnight: + * 86400 seconds per day == 2^7 * 675, which helps us + * replace an expensive div_s64_rem() with a hand-written + * 39-bit modulo on 32-bit architectures. + */ + if (!IS_ENABLED(CONFIG_64BIT)) + ms = (now.tv_sec & 0x7f) * MSEC_PER_SEC + + ((u32)(now.tv_sec >> 7) % 675) * 0x80 * MSEC_PER_SEC; + else + ms = (now.tv_sec % 86400) * MSEC_PER_SEC; + + ms += now.tv_nsec / NSEC_PER_MSEC; + + return ms; +} +EXPORT_SYMBOL_GPL(ktime_get_ms_since_midnight); +#endif + #ifdef CONFIG_NTP_PPS
/** diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c index e5eb8ac4089d..b1c53f2f7bd5 100644 --- a/net/ipv4/icmp.c +++ b/net/ipv4/icmp.c @@ -914,7 +914,6 @@ static bool icmp_echo(struct sk_buff *skb) */ static bool icmp_timestamp(struct sk_buff *skb) { - struct timespec tv; struct icmp_bxm icmp_param; /* * Too short. @@ -923,11 +922,10 @@ static bool icmp_timestamp(struct sk_buff *skb) goto out_err;
/* - * Fill in the current time as ms since midnight UT: + * Fill in the current time as ms since midnight UT, + * this could probably be done faster. */ - getnstimeofday(&tv); - icmp_param.data.times[1] = htonl((tv.tv_sec % 86400) * MSEC_PER_SEC + - tv.tv_nsec / NSEC_PER_MSEC); + icmp_param.data.times[1] = htonl(ktime_get_ms_since_midnight()); icmp_param.data.times[2] = icmp_param.data.times[1]; if (skb_copy_bits(skb, 0, &icmp_param.data.times[0], 4)) BUG(); diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c index bd246792360b..339ce528ecae 100644 --- a/net/ipv4/ip_options.c +++ b/net/ipv4/ip_options.c @@ -58,10 +58,8 @@ void ip_options_build(struct sk_buff *skb, struct ip_options *opt, if (opt->ts_needaddr) ip_rt_get_source(iph+opt->ts+iph[opt->ts+2]-9, skb, rt); if (opt->ts_needtime) { - struct timespec tv; __be32 midtime; - getnstimeofday(&tv); - midtime = htonl((tv.tv_sec % 86400) * MSEC_PER_SEC + tv.tv_nsec / NSEC_PER_MSEC); + midtime = htonl(ktime_get_ms_since_midnight()); memcpy(iph+opt->ts+iph[opt->ts+2]-5, &midtime, 4); } return; @@ -415,10 +413,7 @@ int ip_options_compile(struct net *net, break; } if (timeptr) { - struct timespec tv; - u32 midtime; - getnstimeofday(&tv); - midtime = (tv.tv_sec % 86400) * MSEC_PER_SEC + tv.tv_nsec / NSEC_PER_MSEC; + u32 midtime = ktime_get_ms_since_midnight(); put_unaligned_be32(midtime, timeptr); opt->is_changed = 1; }
Signed-off-by: Fengguang Wu fengguang.wu@intel.com --- timekeeping.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 5611a6d..46b847a 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -847,7 +847,7 @@ time64_t ktime_get_real_seconds(void) EXPORT_SYMBOL_GPL(ktime_get_real_seconds);
#if IS_ENABLED(CONFIG_INET) -u32 ktime_get_ms_of_day(void) +static u32 ktime_get_ms_of_day(void) { struct timekeeper *tk = &tk_core.timekeeper; struct timespec64 now;
Hi Arnd,
[auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
kernel/time/timekeeping.c:850:5: sparse: symbol 'ktime_get_ms_of_day' was not declared. Should it be static?
Please review and possibly fold the followup patch.
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
The can subsystem communicates with user space using a bcm_msg_head header, which contains two timestamps. This is problematic for multiple reasons:
a) The structure layout is currently incompatible between 64-bit user space and 32-bit user space, and cannot work in compat mode (other than x32).
b) The timeval structure layout will change in 32-bit user space when we fix the y2038 overflow problem by redefining time_t to 64-bit, making new 32-bit user space incompatible with the current kernel interface. Cars last a long time and often use old kernels, so the actual users of this code are the most likely ones to migrate to y2038 safe user space.
This tries to work around part of the problem by changing the publicly visible user interface in the header, but not the binary interface. Fortunately, the values passed around in the structure are relative times and do not actually suffer from the y2038 overflow, so 32-bit is enough here.
We replace the use of 'struct timeval' with a newly defined 'struct bcm_timeval' that uses the exact same binary layout as before and that still suffers from problem a) but not problem b).
The downside of this approach is that any user space program that currently assigns a timeval structure to these members rather than writing the tv_sec/tv_usec portions individually will suffer a compile-time error when built with an updated kernel header. Fixing this error makes it work fine with old and new headers though.
We could address problem a) by using '__u32' or 'int' members rather than 'long', but that would have a more significant downside in also breaking support for all existing 64-bit user binaries that might be using this interface, which is likely not acceptable.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Oliver Hartkopp socketcan@hartkopp.net Cc: Marc Kleine-Budde mkl@pengutronix.de Cc: linux-can@vger.kernel.org Cc: linux-api@vger.kernel.org --- include/uapi/linux/can/bcm.h | 7 ++++++- net/can/bcm.c | 15 ++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/can/bcm.h b/include/uapi/linux/can/bcm.h index 89ddb9dc9bdf..7a291dc1ff15 100644 --- a/include/uapi/linux/can/bcm.h +++ b/include/uapi/linux/can/bcm.h @@ -47,6 +47,11 @@ #include <linux/types.h> #include <linux/can.h>
+struct bcm_timeval { + long tv_sec; + long tv_usec; +}; + /** * struct bcm_msg_head - head of messages to/from the broadcast manager * @opcode: opcode, see enum below. @@ -62,7 +67,7 @@ struct bcm_msg_head { __u32 opcode; __u32 flags; __u32 count; - struct timeval ival1, ival2; + struct bcm_timeval ival1, ival2; canid_t can_id; __u32 nframes; struct can_frame frames[0]; diff --git a/net/can/bcm.c b/net/can/bcm.c index a1ba6875c2a2..6863310d6973 100644 --- a/net/can/bcm.c +++ b/net/can/bcm.c @@ -96,7 +96,7 @@ struct bcm_op { canid_t can_id; u32 flags; unsigned long frames_abs, frames_filtered; - struct timeval ival1, ival2; + struct bcm_timeval ival1, ival2; struct hrtimer timer, thrtimer; struct tasklet_struct tsklet, thrtsklet; ktime_t rx_stamp, kt_ival1, kt_ival2, kt_lastmsg; @@ -131,6 +131,11 @@ static inline struct bcm_sock *bcm_sk(const struct sock *sk) return (struct bcm_sock *)sk; }
+static inline ktime_t bcm_timeval_to_ktime(struct bcm_timeval tv) +{ + return ktime_set(tv.tv_sec, tv.tv_usec * NSEC_PER_USEC); +} + #define CFSIZ sizeof(struct can_frame) #define OPSIZ sizeof(struct bcm_op) #define MHSIZ sizeof(struct bcm_msg_head) @@ -953,8 +958,8 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, op->count = msg_head->count; op->ival1 = msg_head->ival1; op->ival2 = msg_head->ival2; - op->kt_ival1 = timeval_to_ktime(msg_head->ival1); - op->kt_ival2 = timeval_to_ktime(msg_head->ival2); + op->kt_ival1 = bcm_timeval_to_ktime(msg_head->ival1); + op->kt_ival2 = bcm_timeval_to_ktime(msg_head->ival2);
/* disable an active timer due to zero values? */ if (!op->kt_ival1.tv64 && !op->kt_ival2.tv64) @@ -1134,8 +1139,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, /* set timer value */ op->ival1 = msg_head->ival1; op->ival2 = msg_head->ival2; - op->kt_ival1 = timeval_to_ktime(msg_head->ival1); - op->kt_ival2 = timeval_to_ktime(msg_head->ival2); + op->kt_ival1 = bcm_timeval_to_ktime(msg_head->ival1); + op->kt_ival2 = bcm_timeval_to_ktime(msg_head->ival2);
/* disable an active timer due to zero value? */ if (!op->kt_ival1.tv64)
Hello Arnd,
thanks for picking up this y2038 api issue.
On 09/30/2015 01:26 PM, Arnd Bergmann wrote:
The can subsystem communicates with user space using a bcm_msg_head header, which contains two timestamps. This is problematic for multiple reasons:
a) The structure layout is currently incompatible between 64-bit user space and 32-bit user space, and cannot work in compat mode (other than x32).
b) The timeval structure layout will change in 32-bit user space when we fix the y2038 overflow problem by redefining time_t to 64-bit, making new 32-bit user space incompatible with the current kernel interface. Cars last a long time and often use old kernels, so the actual users of this code are the most likely ones to migrate to y2038 safe user space.
This tries to work around part of the problem by changing the publicly visible user interface in the header, but not the binary interface. Fortunately, the values passed around in the structure are relative times and do not actually suffer from the y2038 overflow, so 32-bit is enough here.
We replace the use of 'struct timeval' with a newly defined 'struct bcm_timeval' that uses the exact same binary layout as before and that still suffers from problem a) but not problem b).
The downside of this approach is that any user space program that currently assigns a timeval structure to these members rather than writing the tv_sec/tv_usec portions individually will suffer a compile-time error when built with an updated kernel header. Fixing this error makes it work fine with old and new headers though.
I double checked some (more) BCM applications I have access to.
E.g. https://github.com/linux-can/can-tests
When you do a 'git grep ival1' there you get something like
tst-bcm-cycle.c: msg.msg_head.ival1.tv_sec = 1; tst-bcm-cycle.c: msg.msg_head.ival1.tv_usec = 0; tst-bcm-cycle.c: msg.msg_head.ival1.tv_sec = 0; tst-bcm-cycle.c: msg.msg_head.ival1.tv_usec = 0; tst-bcm-dump.c: msg.msg_head.ival1.tv_sec = timeout / 1000000; tst-bcm-dump.c: msg.msg_head.ival1.tv_usec = timeout % 1000000; (..)
So the usual way to assign values to ival1 and ival2 is NOT to assign an existing struct timeval but to directly assign its tv_[u]sec elements.
I applied your bcm.h changes to my local can-tests tree and it compiles without any problems - as expected. I don't see any serious drawback with your idea. I wonder whether developers would ever notice this change ...
We could address problem a) by using '__u32' or 'int' members rather than 'long', but that would have a more significant downside in also breaking support for all existing 64-bit user binaries that might be using this interface, which is likely not acceptable.
Indeed.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Oliver Hartkopp socketcan@hartkopp.net
Thanks for your good suggestion to make the BCM API y2038 proof!
Acked-by: Oliver Hartkopp socketcan@hartkopp.net
Cc: Marc Kleine-Budde mkl@pengutronix.de Cc: linux-can@vger.kernel.org Cc: linux-api@vger.kernel.org
include/uapi/linux/can/bcm.h | 7 ++++++- net/can/bcm.c | 15 ++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/can/bcm.h b/include/uapi/linux/can/bcm.h index 89ddb9dc9bdf..7a291dc1ff15 100644 --- a/include/uapi/linux/can/bcm.h +++ b/include/uapi/linux/can/bcm.h @@ -47,6 +47,11 @@ #include <linux/types.h> #include <linux/can.h> +struct bcm_timeval {
- long tv_sec;
- long tv_usec;
+};
/**
- struct bcm_msg_head - head of messages to/from the broadcast manager
- @opcode: opcode, see enum below.
@@ -62,7 +67,7 @@ struct bcm_msg_head { __u32 opcode; __u32 flags; __u32 count;
- struct timeval ival1, ival2;
- struct bcm_timeval ival1, ival2; canid_t can_id; __u32 nframes; struct can_frame frames[0];
diff --git a/net/can/bcm.c b/net/can/bcm.c index a1ba6875c2a2..6863310d6973 100644 --- a/net/can/bcm.c +++ b/net/can/bcm.c @@ -96,7 +96,7 @@ struct bcm_op { canid_t can_id; u32 flags; unsigned long frames_abs, frames_filtered;
- struct timeval ival1, ival2;
- struct bcm_timeval ival1, ival2; struct hrtimer timer, thrtimer; struct tasklet_struct tsklet, thrtsklet; ktime_t rx_stamp, kt_ival1, kt_ival2, kt_lastmsg;
@@ -131,6 +131,11 @@ static inline struct bcm_sock *bcm_sk(const struct sock *sk) return (struct bcm_sock *)sk; } +static inline ktime_t bcm_timeval_to_ktime(struct bcm_timeval tv) +{
- return ktime_set(tv.tv_sec, tv.tv_usec * NSEC_PER_USEC);
+}
#define CFSIZ sizeof(struct can_frame) #define OPSIZ sizeof(struct bcm_op) #define MHSIZ sizeof(struct bcm_msg_head) @@ -953,8 +958,8 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, op->count = msg_head->count; op->ival1 = msg_head->ival1; op->ival2 = msg_head->ival2;
op->kt_ival1 = timeval_to_ktime(msg_head->ival1);
op->kt_ival2 = timeval_to_ktime(msg_head->ival2);
op->kt_ival1 = bcm_timeval_to_ktime(msg_head->ival1);
op->kt_ival2 = bcm_timeval_to_ktime(msg_head->ival2);
/* disable an active timer due to zero values? */ if (!op->kt_ival1.tv64 && !op->kt_ival2.tv64) @@ -1134,8 +1139,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, /* set timer value */ op->ival1 = msg_head->ival1; op->ival2 = msg_head->ival2;
op->kt_ival1 = timeval_to_ktime(msg_head->ival1);
op->kt_ival2 = timeval_to_ktime(msg_head->ival2);
op->kt_ival1 = bcm_timeval_to_ktime(msg_head->ival1);
op->kt_ival2 = bcm_timeval_to_ktime(msg_head->ival2);
/* disable an active timer due to zero value? */ if (!op->kt_ival1.tv64)
On Monday 05 October 2015 20:51:08 Oliver Hartkopp wrote:
I double checked some (more) BCM applications I have access to.
E.g. https://github.com/linux-can/can-tests
When you do a 'git grep ival1' there you get something like
tst-bcm-cycle.c: msg.msg_head.ival1.tv_sec = 1; tst-bcm-cycle.c: msg.msg_head.ival1.tv_usec = 0; tst-bcm-cycle.c: msg.msg_head.ival1.tv_sec = 0; tst-bcm-cycle.c: msg.msg_head.ival1.tv_usec = 0; tst-bcm-dump.c: msg.msg_head.ival1.tv_sec = timeout / 1000000; tst-bcm-dump.c: msg.msg_head.ival1.tv_usec = timeout % 1000000; (..)
So the usual way to assign values to ival1 and ival2 is NOT to assign an existing struct timeval but to directly assign its tv_[u]sec elements.
Ok, very good.
I applied your bcm.h changes to my local can-tests tree and it compiles without any problems - as expected. I don't see any serious drawback with your idea. I wonder whether developers would ever notice this change ...
We could address problem a) by using '__u32' or 'int' members rather than 'long', but that would have a more significant downside in also breaking support for all existing 64-bit user binaries that might be using this interface, which is likely not acceptable.
Indeed.
Signed-off-by: Arnd Bergmann arnd@arndb.de Cc: Oliver Hartkopp socketcan@hartkopp.net
Thanks for your good suggestion to make the BCM API y2038 proof!
Acked-by: Oliver Hartkopp socketcan@hartkopp.net
Thanks.
What is the normal path for CAN patches? Should I resend with your Ack and without the RFC for Marc to pick it up?
Arnd
From: Arnd Bergmann arnd@arndb.de Date: Wed, 30 Sep 2015 13:26:30 +0200
This is a set of changes for network drivers and core code to get rid of the use of time_t and derived data structures.
Applied patches #1-#10 to net-next, using the updated v2 version of the zatm change.