linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/12] net: assorted y2038 changes
@ 2015-09-30 11:26 Arnd Bergmann
  2015-09-30 11:26 ` [PATCH 12/12] [RFC] can: avoid using timeval for uapi Arnd Bergmann
       [not found] ` <1443612402-3000775-1-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
  0 siblings, 2 replies; 8+ messages in thread
From: Arnd Bergmann @ 2015-09-30 11:26 UTC (permalink / raw)
  To: netdev
  Cc: Arnd Bergmann, y2038, linux-api, linux-atm-general,
	linux-wireless, linux-kernel, linux-can, linux-sctp, coreteam,
	intel-wired-lan, netfilter-devel, David S. Miller

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


-- 
2.1.0.rc2

_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 12/12] [RFC] can: avoid using timeval for uapi
  2015-09-30 11:26 [PATCH 00/12] net: assorted y2038 changes Arnd Bergmann
@ 2015-09-30 11:26 ` Arnd Bergmann
       [not found]   ` <1443612402-3000775-13-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
       [not found] ` <1443612402-3000775-1-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
  1 sibling, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2015-09-30 11:26 UTC (permalink / raw)
  To: netdev
  Cc: Arnd Bergmann, y2038, Oliver Hartkopp, linux-kernel, linux-can,
	Marc Kleine-Budde, linux-api, David S. Miller

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)
-- 
2.1.0.rc2

_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 00/12] net: assorted y2038 changes
       [not found] ` <1443612402-3000775-1-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
@ 2015-10-05 10:17   ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2015-10-05 10:17 UTC (permalink / raw)
  To: arnd-r2nGTMty4D4
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, y2038-cunTk1MwBs8s++Sfvej+rw,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	coreteam-Cap9r6Oaw4JrovVCs/uTlw,
	intel-wired-lan-qjLDD68F18P21nG7glBr7A,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-atm-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-can-u79uwXL29TY76Z2rM5mHXA,
	linux-sctp-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netfilter-devel-u79uwXL29TY76Z2rM5mHXA

From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 12/12] [RFC] can: avoid using timeval for uapi
       [not found]   ` <1443612402-3000775-13-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
@ 2015-10-05 18:51     ` Oliver Hartkopp
  2015-10-06  8:32       ` Arnd Bergmann
  0 siblings, 1 reply; 8+ messages in thread
From: Oliver Hartkopp @ 2015-10-05 18:51 UTC (permalink / raw)
  To: Arnd Bergmann, netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: y2038-cunTk1MwBs8s++Sfvej+rw, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	David S. Miller, Marc Kleine-Budde,
	linux-can-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA

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-r2nGTMty4D4@public.gmane.org>
> Cc: Oliver Hartkopp <socketcan-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>

Thanks for your good suggestion to make the BCM API y2038 proof!

Acked-by: Oliver Hartkopp <socketcan-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>

> Cc: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: linux-can-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.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)
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 12/12] [RFC] can: avoid using timeval for uapi
  2015-10-05 18:51     ` Oliver Hartkopp
@ 2015-10-06  8:32       ` Arnd Bergmann
  2015-10-06  9:05         ` Marc Kleine-Budde
  0 siblings, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2015-10-06  8:32 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: y2038, netdev, linux-kernel, linux-can, Marc Kleine-Budde,
	linux-api, David S. Miller

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
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 12/12] [RFC] can: avoid using timeval for uapi
  2015-10-06  8:32       ` Arnd Bergmann
@ 2015-10-06  9:05         ` Marc Kleine-Budde
  2015-10-06  9:18           ` Arnd Bergmann
  0 siblings, 1 reply; 8+ messages in thread
From: Marc Kleine-Budde @ 2015-10-06  9:05 UTC (permalink / raw)
  To: Arnd Bergmann, Oliver Hartkopp
  Cc: netdev, y2038, linux-kernel, David S. Miller, linux-can,
	linux-api

[-- Attachment #1: Type: text/plain, Size: 2188 bytes --]

On 10/06/2015 10:32 AM, Arnd Bergmann wrote:
> 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?

You can add my:

Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>

add upstream the 2038 fixes as a block.

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 12/12] [RFC] can: avoid using timeval for uapi
  2015-10-06  9:05         ` Marc Kleine-Budde
@ 2015-10-06  9:18           ` Arnd Bergmann
  2015-10-06  9:37             ` Marc Kleine-Budde
  0 siblings, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2015-10-06  9:18 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Oliver Hartkopp, netdev, y2038, linux-kernel, David S. Miller,
	linux-can, linux-api

On Tuesday 06 October 2015 11:05:32 Marc Kleine-Budde wrote:
> On 10/06/2015 10:32 AM, Arnd Bergmann wrote:
> > 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?
> 
> You can add my:
> 
> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
> 
> add upstream the 2038 fixes as a block.

Davem already picked up the first 10 of the series. If you don't
mind, I'd prefer if you could take this one into your tree so I
have it off my list.
I have 200 other patches in various states and more getting added.

	Arnd

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 12/12] [RFC] can: avoid using timeval for uapi
  2015-10-06  9:18           ` Arnd Bergmann
@ 2015-10-06  9:37             ` Marc Kleine-Budde
  0 siblings, 0 replies; 8+ messages in thread
From: Marc Kleine-Budde @ 2015-10-06  9:37 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Oliver Hartkopp, netdev-u79uwXL29TY76Z2rM5mHXA,
	y2038-cunTk1MwBs8s++Sfvej+rw, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	David S. Miller, linux-can-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 715 bytes --]

On 10/06/2015 11:18 AM, Arnd Bergmann wrote:
>> Acked-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>
>> add upstream the 2038 fixes as a block.
> 
> Davem already picked up the first 10 of the series. If you don't
> mind, I'd prefer if you could take this one into your tree so I
> have it off my list.
> I have 200 other patches in various states and more getting added.

Ok. Consider it done.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-10-06  9:37 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-30 11:26 [PATCH 00/12] net: assorted y2038 changes Arnd Bergmann
2015-09-30 11:26 ` [PATCH 12/12] [RFC] can: avoid using timeval for uapi Arnd Bergmann
     [not found]   ` <1443612402-3000775-13-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
2015-10-05 18:51     ` Oliver Hartkopp
2015-10-06  8:32       ` Arnd Bergmann
2015-10-06  9:05         ` Marc Kleine-Budde
2015-10-06  9:18           ` Arnd Bergmann
2015-10-06  9:37             ` Marc Kleine-Budde
     [not found] ` <1443612402-3000775-1-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
2015-10-05 10:17   ` [PATCH 00/12] net: assorted y2038 changes David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).