[systemd-devel] [PATCH 1/3] socket: Add Support for TCP keep alive variables
susant at redhat.com
Thu Aug 14 04:14:51 PDT 2014
On 08/14/2014 06:13 AM, Lennart Poettering wrote:
> On Tue, 29.07.14 23:10, Susant Sahani (susant at redhat.com) wrote:
>> tcp_keepalive_time: The number of seconds a connection needs to be
>> idle before TCP begins sending out keep-alive probes.
> Looks pretty OK.
>> + <term><varname>KeepAliveTime=</varname></term>
>> + <listitem><para>Takes time (in seconds) as
>> argument . The connection needs to remain
> ^^^^^ there's a spurious space too much here...
>> + idle before TCP starts sending keepalive probes. This controls the TCP_KEEPIDLE
>> + socket option (see
>> + </varlistentry>
> Could you indent this like the rest of the settings, please?
>> + SD_BUS_PROPERTY("KeepAliveTime", "t", bus_property_get_usec, offsetof(Socket, keep_alive_time), SD_BUS_VTABLE_PROPERTY_CONST),
>> + SD_BUS_PROPERTY("KeepAliveInterval", "t", bus_property_get_usec, offsetof(Socket, keep_alive_interval), SD_BUS_VTABLE_PROPERTY_CONST),
>> + SD_BUS_PROPERTY("KeepAliveProbes", "i", bus_property_get_int,
>> offsetof(Socket, keep_alive_cnt), SD_BUS_VTABLE_PROPERTY_CONST),
> This should really be an "u" and use bus_property_get_unsigned(), no? I
> mean, there is no negative count possible, is there?
>> + if(s->keep_alive_time)
>> + fprintf(f,
>> + "%sKeepAliveTime: %lo\n",
>> + prefix, s->keep_alive_time / USEC_PER_SEC);
> Please format this with format_timespan()!
made the changes
> Otherwise looks good!
Re-sending Both the patches . Thanks for reviewing.
More information about the systemd-devel