Re: [Intel-wired-lan] [RFC v2 net-next 01/10] net: Add a new socket option for a future transmit time.
On Mon, Feb 12, 2018 at 02:39:06PM -0800, Jesus Sanchez-Palencia wrote: > On 01/18/2018 12:42 AM, Miroslav Lichvar wrote: > > Please keep in mind that the PHCs and the system clock don't have to > > be synchronized to each other. If I understand the rest of the series > > correctly, there is an assumption that the PHCs are keeping time in > > TAI and CLOCK_TAI can be used as a fallback. > > Just to double-check, imagine that I've configured the qdisc for > SW best-effort and with clockid CLOCK_REALTIME. When it receives a > packet with the clockid of a /dev/ptpX, the qdisc should just drop that > packet, right? Yes, I think it should drop it. The kernel does not know the offset between the two clocks (they don't even have to be synchronized), so it cannot convert a PHC-based TX time to the system time. -- Miroslav Lichvar
Re: [Intel-wired-lan] [RFC v2 net-next 01/10] net: Add a new socket option for a future transmit time.
Hi, On 01/18/2018 12:42 AM, Miroslav Lichvar wrote: > On Wed, Jan 17, 2018 at 03:06:12PM -0800, Jesus Sanchez-Palencia wrote: >> From: Richard Cochran >> >> This patch introduces SO_TXTIME. User space enables this option in >> order to pass a desired future transmit time in a CMSG when calling >> sendmsg(2). >> >> A new field is added to struct sockcm_cookie, and the tstamp from >> skbuffs will be used later on. > > In the discussion about the v1 patchset, there was a question if the > cmsg should include a clockid_t. Without that, how can an application > prevent the packet from being sent using an incorrect clock, e.g. > the system clock when it expects it to be a PHC, or a different PHC > when the socket is not bound to a specific interface? > > At least in some applications it would be preferred to not sent a > packet at all instead of sending it at a wrong time. > > Please keep in mind that the PHCs and the system clock don't have to > be synchronized to each other. If I understand the rest of the series > correctly, there is an assumption that the PHCs are keeping time in > TAI and CLOCK_TAI can be used as a fallback. Just to double-check, imagine that I've configured the qdisc for SW best-effort and with clockid CLOCK_REALTIME. When it receives a packet with the clockid of a /dev/ptpX, the qdisc should just drop that packet, right? Or would this block any use-cases that I couldn't think of ? Thanks, Jesus
Re: [Intel-wired-lan] [RFC v2 net-next 01/10] net: Add a new socket option for a future transmit time.
Hi, On 02/01/2018 01:27 AM, Miroslav Lichvar wrote: > On Wed, Jan 31, 2018 at 04:49:36PM -0800, Jesus Sanchez-Palencia wrote: >> On 01/18/2018 09:13 AM, Richard Cochran wrote: >>> Right, the clockid_t should be passed in through the CMSG along with >>> the time. >> >> While implementing this today it crossed my mind that why don't we have the >> clockid_t set per socket (e.g. as an argument to SO_TXTIME) instead of per >> packet? > > I suspect that might have an impact on the performance. Even if the > application doesn't use sendmmsg(), it would possibly have to call > setsockopt() before each sendmsg() to change the clockid_t, right? Yes. On the other hand, for applications that will be using only 1 clockid_t, keeping it per packet will also have an impact as we'll be copying the same value from the cmsg cookie into sk_buffs over and over. > > If clockid_t could be set per packet, a special value could be used > to allow sending on interfaces that don't support it. > >> The only use-case that we could think of that would be 'blocked' was using >> sendmmsg() to send a packet to different interfaces with a single syscall, >> but >> I'm not sure how common that is. > > The SO_TXTIME option will make sendmmsg() useful in applications where > it wasn't before. For instance, an NTP server will be able to batch > multiple responses as their transmit timestamps can be set accurately > in advance and it's no longer necessary to send the responses as soon > as they are assembled. > > I think it would be nice the sendmmsg() calls didn't have to be split > by clockid_t. OK, fair enough. I will keep it per-packet for now as initially agreed and we can revisit this later if needed. Thanks, Jesus
Re: [Intel-wired-lan] [RFC v2 net-next 01/10] net: Add a new socket option for a future transmit time.
On Wed, Jan 31, 2018 at 04:49:36PM -0800, Jesus Sanchez-Palencia wrote: > On 01/18/2018 09:13 AM, Richard Cochran wrote: > > Right, the clockid_t should be passed in through the CMSG along with > > the time. > > While implementing this today it crossed my mind that why don't we have the > clockid_t set per socket (e.g. as an argument to SO_TXTIME) instead of per > packet? I suspect that might have an impact on the performance. Even if the application doesn't use sendmmsg(), it would possibly have to call setsockopt() before each sendmsg() to change the clockid_t, right? If clockid_t could be set per packet, a special value could be used to allow sending on interfaces that don't support it. > The only use-case that we could think of that would be 'blocked' was using > sendmmsg() to send a packet to different interfaces with a single syscall, but > I'm not sure how common that is. The SO_TXTIME option will make sendmmsg() useful in applications where it wasn't before. For instance, an NTP server will be able to batch multiple responses as their transmit timestamps can be set accurately in advance and it's no longer necessary to send the responses as soon as they are assembled. I think it would be nice the sendmmsg() calls didn't have to be split by clockid_t. -- Miroslav Lichvar
Re: [Intel-wired-lan] [RFC v2 net-next 01/10] net: Add a new socket option for a future transmit time.
On Wed, Jan 31, 2018 at 04:49:36PM -0800, Jesus Sanchez-Palencia wrote: > While implementing this today it crossed my mind that why don't we have the > clockid_t set per socket (e.g. as an argument to SO_TXTIME) instead of per > packet? Sounds good to me. Thanks, Richard
Re: [Intel-wired-lan] [RFC v2 net-next 01/10] net: Add a new socket option for a future transmit time.
Hi, On 01/18/2018 09:13 AM, Richard Cochran wrote: > On Thu, Jan 18, 2018 at 09:42:27AM +0100, Miroslav Lichvar wrote: >> In the discussion about the v1 patchset, there was a question if the >> cmsg should include a clockid_t. Without that, how can an application >> prevent the packet from being sent using an incorrect clock, e.g. >> the system clock when it expects it to be a PHC, or a different PHC >> when the socket is not bound to a specific interface? > > Right, the clockid_t should be passed in through the CMSG along with > the time. While implementing this today it crossed my mind that why don't we have the clockid_t set per socket (e.g. as an argument to SO_TXTIME) instead of per packet? The only use-case that we could think of that would be 'blocked' was using sendmmsg() to send a packet to different interfaces with a single syscall, but I'm not sure how common that is. What do you think? Thanks, Jesus > > Thanks, > Richard >
Re: [Intel-wired-lan] [RFC v2 net-next 01/10] net: Add a new socket option for a future transmit time.
On Wed, Jan 24, 2018 at 02:46:24PM -0800, Vinicius Costa Gomes wrote: > The only robust way that we could think of about keeping the the packets > in order for the device queue is re-ordering packets in the Qdisc. Right, but you cannot afford the overhead of the timerqueue when using HW offload, when the HW device sits on a PCIe bus. Many serious TSN applications (like industrial controls) will want to have just one packet queued, readying the next one just in time for the next deadline. The control loops are sensitive to the feedback interval. > Even if we reach a decision that the Qdisc should not re-order packets > (we wouldn't have any dependency on hrtimers in the offload case, as you > pointed out), we still need hrtimers for the software implementation. Fine. > So, I guess, the problem remains, if it's possible for the user to > express a /dev/ptp* clock, what should we do? Thinking a bit more, it doesn't make sense to have a user choice for the HW offloading case. The clock should implicitly be the device clock, always. Using any other clock would make no sense. Thanks, Richard
Re: [Intel-wired-lan] [RFC v2 net-next 01/10] net: Add a new socket option for a future transmit time.
On Thu, Jan 25, 2018 at 10:12:25AM +0100, Miroslav Lichvar wrote: > Do I understand it correctly that no other interface is using > nanoseconds since 1970? We probably don't have to worry about year > 2262 yet, but wouldn't it be better to make it consistent with the > timestamping API using timespec? Or is it just better to avoid the > 64/32-bit mess of time_t? I prefer a single 64 bit nanoseconds field: - Applications won't have to convert to timespec. - Avoids the time_t issue. Thanks, Richard
Re: [Intel-wired-lan] [RFC v2 net-next 01/10] net: Add a new socket option for a future transmit time.
On Fri, Jan 19, 2018 at 06:09:15PM -0800, Richard Cochran wrote: > On Fri, Jan 19, 2018 at 04:15:46PM -0500, Willem de Bruijn wrote: > > > + if (cmsg->cmsg_len != CMSG_LEN(sizeof(ktime_t))) > > > + return -EINVAL; > > > > I don't see any existing reference to ktime_t in include/uapi. Just use a > > s64? > > Agreed. I didn't see the point of switching to ktime, either. Do I understand it correctly that no other interface is using nanoseconds since 1970? We probably don't have to worry about year 2262 yet, but wouldn't it be better to make it consistent with the timestamping API using timespec? Or is it just better to avoid the 64/32-bit mess of time_t? -- Miroslav Lichvar
Re: [Intel-wired-lan] [RFC v2 net-next 01/10] net: Add a new socket option for a future transmit time.
Hi Richard, Richard Cochran writes: > On Tue, Jan 23, 2018 at 01:22:37PM -0800, Vinicius Costa Gomes wrote: >> What I think would be the ideal scenario would be if the clockid >> parameter to the TBS Qdisc would not be necessary (if offload was >> enabled), but that's not quite possible right now, because there's no >> support for using the hrtimer infrastructure with dynamic clocks >> (/dev/ptp*). > > We don't need hrtimer for HW offloading. Just enqueue the packets. I > thought we agreed that user space get the ordering correct. In fact, > davem insisted on it, IIRC. About the ordering of packets, From here [1], there are 3 clear points (in my understanding): 1. Re-ordering of TX descriptors on the device queue should/must not happen; 2. Out of order requests are an error; 3. Timestamps in the past are an error; The only robust way that we could think of about keeping the the packets in order for the device queue is re-ordering packets in the Qdisc. We tried to reach out for confirmation [2] of this understanding but didn't receive any word. Even if we reach a decision that the Qdisc should not re-order packets (we wouldn't have any dependency on hrtimers in the offload case, as you pointed out), we still need hrtimers for the software implementation. So, I guess, the problem remains, if it's possible for the user to express a /dev/ptp* clock, what should we do? > > Thanks, > Richard Cheers, -- Vinicius [1] https://patchwork.ozlabs.org/comment/1770302/ [2] https://patchwork.ozlabs.org/comment/1816492/q
Re: [Intel-wired-lan] [RFC v2 net-next 01/10] net: Add a new socket option for a future transmit time.
On Tue, Jan 23, 2018 at 01:22:37PM -0800, Vinicius Costa Gomes wrote: > What I think would be the ideal scenario would be if the clockid > parameter to the TBS Qdisc would not be necessary (if offload was > enabled), but that's not quite possible right now, because there's no > support for using the hrtimer infrastructure with dynamic clocks > (/dev/ptp*). We don't need hrtimer for HW offloading. Just enqueue the packets. I thought we agreed that user space get the ordering correct. In fact, davem insisted on it, IIRC. Thanks, Richard
Re: [Intel-wired-lan] [RFC v2 net-next 01/10] net: Add a new socket option for a future transmit time.
Hi, Miroslav Lichvar writes: > On Wed, Jan 17, 2018 at 03:06:12PM -0800, Jesus Sanchez-Palencia wrote: >> From: Richard Cochran >> >> This patch introduces SO_TXTIME. User space enables this option in >> order to pass a desired future transmit time in a CMSG when calling >> sendmsg(2). >> >> A new field is added to struct sockcm_cookie, and the tstamp from >> skbuffs will be used later on. > > In the discussion about the v1 patchset, there was a question if the > cmsg should include a clockid_t. Without that, how can an application > prevent the packet from being sent using an incorrect clock, e.g. > the system clock when it expects it to be a PHC, or a different PHC > when the socket is not bound to a specific interface? > > At least in some applications it would be preferred to not sent a > packet at all instead of sending it at a wrong time. Including the clockid in a CMSG field does make sense. Will add it in the next version of this series. What I think would be the ideal scenario would be if the clockid parameter to the TBS Qdisc would not be necessary (if offload was enabled), but that's not quite possible right now, because there's no support for using the hrtimer infrastructure with dynamic clocks (/dev/ptp*). What I am thinking is to keep the clockid parameter for the Qdisc (and add support for expressing the clockid in friendlier ways, as requested later in this thread), but I can't think of a way to add support for using /dev/ptp* clocks without first having hrtimer support them. And the behavior would be to drop any packets with a clockid not matching the Qdisc clockid. How does this sound? > > Please keep in mind that the PHCs and the system clock don't have to > be synchronized to each other. If I understand the rest of the series > correctly, there is an assumption that the PHCs are keeping time in > TAI and CLOCK_TAI can be used as a fallback. You understand correctly, that's because of whole hrtimer issue. > > -- > Miroslav Lichvar Cheers, -- Vinicius
Re: [Intel-wired-lan] [RFC v2 net-next 01/10] net: Add a new socket option for a future transmit time.
On Thu, Jan 18, 2018 at 09:42:27AM +0100, Miroslav Lichvar wrote: > In the discussion about the v1 patchset, there was a question if the > cmsg should include a clockid_t. Without that, how can an application > prevent the packet from being sent using an incorrect clock, e.g. > the system clock when it expects it to be a PHC, or a different PHC > when the socket is not bound to a specific interface? Right, the clockid_t should be passed in through the CMSG along with the time. Thanks, Richard
Re: [Intel-wired-lan] [RFC v2 net-next 01/10] net: Add a new socket option for a future transmit time.
On Wed, Jan 17, 2018 at 03:06:12PM -0800, Jesus Sanchez-Palencia wrote: > From: Richard Cochran > > This patch introduces SO_TXTIME. User space enables this option in > order to pass a desired future transmit time in a CMSG when calling > sendmsg(2). > > A new field is added to struct sockcm_cookie, and the tstamp from > skbuffs will be used later on. In the discussion about the v1 patchset, there was a question if the cmsg should include a clockid_t. Without that, how can an application prevent the packet from being sent using an incorrect clock, e.g. the system clock when it expects it to be a PHC, or a different PHC when the socket is not bound to a specific interface? At least in some applications it would be preferred to not sent a packet at all instead of sending it at a wrong time. Please keep in mind that the PHCs and the system clock don't have to be synchronized to each other. If I understand the rest of the series correctly, there is an assumption that the PHCs are keeping time in TAI and CLOCK_TAI can be used as a fallback. -- Miroslav Lichvar
