[systemd-devel] [PATCH 1/3] socket: Add Support for TCP keep alive variables

[Susant Sahani]

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.
>
>> ---

>>
>>                           <varlistentry>
>> +                          <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...

Removed .
>
>
>> +                          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?

Ok

>
>> +        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?

Yes :)

>
>> +        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!
>
> Lennart
>

Re-sending Both the patches . Thanks for reviewing.

Susant