From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
Subject: Re: [PATCH v1 net-next 02/14] net: Add a new socket option for a
 future transmit time.
Date: Thu, 28 Jun 2018 11:33:11 -0700
Message-ID: <e5b466c4-1846-0626-239a-f0e1a4f1f7c4@intel.com>
References: <20180627215950.6719-1-jesus.sanchez-palencia@intel.com>
 <20180627215950.6719-3-jesus.sanchez-palencia@intel.com>
 <CAF=yD-KY60-Hi2_BSb2jOR7W91wOJQtSQ11WSDEDVpY6wx+aUw@mail.gmail.com>
 <CAF=yD-KccMgPRL67YY9X3U6jP9E1WVc3C=b=wnAZvW8URs1FMQ@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: Network Development <netdev@vger.kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        jan.altenberg@linutronix.de,
        Vinicius Gomes <vinicius.gomes@intel.com>,
        kurt.kanzenbach@linutronix.de, Henrik Austad <henrik@austad.us>,
        Richard Cochran <richardcochran@gmail.com>,
        Levi Pearson <levi.pearson@harman.com>,
        ilias.apalodimas@linaro.org, ivan.khoronzhuk@linaro.org,
        Miroslav Lichvar <mlichvar@redhat.com>,
        Willem de Bruijn <willemb@google.com>,
        Jamal Hadi Salim <jhs@mojatatu.com>,
        Cong Wang <xiyou.wangcong@gmail.com>,
        =?UTF-8?B?SmnFmcOtIFDDrXJrbw==?= <jiri@resnulli.us>,
        Richard Cochran <rcochran@linutronix.de>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mga04.intel.com ([192.55.52.120]:18899 "EHLO mga04.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1030539AbeF1SiD (ORCPT <rfc822;netdev@vger.kernel.org>);
        Thu, 28 Jun 2018 14:38:03 -0400
In-Reply-To: <CAF=yD-KccMgPRL67YY9X3U6jP9E1WVc3C=b=wnAZvW8URs1FMQ@mail.gmail.com>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

Hi Willem,


On 06/28/2018 07:40 AM, Willem de Bruijn wrote:
> On Thu, Jun 28, 2018 at 10:26 AM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
>>
>> On Wed, Jun 27, 2018 at 6:08 PM Jesus Sanchez-Palencia
>> <jesus.sanchez-palencia@intel.com> wrote:
>>>
>>> From: Richard Cochran <rcochran@linutronix.de>
>>>
>>> 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). The argument to this socket option is a 6-bytes long struct
>>> defined as:
>>>
>>> struct sock_txtime {
>>>         clockid_t       clockid;
>>>         u16             flags;
>>> };
>>
>> clockid_t is __kernel_clockid_t is int is a variable length field.
>> Please use fixed length fields.
> 
> Sorry, int is fine, of course, and clockid_t is used between userspace and
> kernel already.


Great. So, in addition to the other feedback in sock.c, what I'm thinking here
for the v2 is:

- move this struct to and the flags definition (as enums) to
include/uapi/linux/net_tstamp.h;

- keep clockid as a clockid_t and increase flags to u32 since this already takes
8 bytes in total anyway;

- reduce sk_clockid and sk_txtime_flags from struct sock from a u16 to a u8 each.


Thanks,
Jesus



> 
>> Also, as MAX_CLOCKS is 16, only 4 bits are needed. A single u16
>> is probably sufficient as cmsg argument. To future proof, a u32 will
>> allow for more
>> than 4 flags. But in struct sock, 16 bits should be sufficient to
>> encode both clock id
>> and flags.