Re: [PATCH] libceph: use keepalive2 to verify the mon session is alive

[Ilya Dryomov]

On Wed, Sep 2, 2015 at 5:22 AM, Yan, Zheng  wrote:
> timespec_to_jiffies() does not work this way. it convert time delta in form 
> of timespec to time delta in form of jiffies.

Ah sorry, con->last_keepalive_ack is a realtime timespec from
userspace.

>
> I will updated the patch according  to the rest comments .

I still want ceph_con_keepalive_expired() to handle interval == 0
internally, so that opt->monc_ping_timeout can be passed without any
checks in the caller.

I also noticed you added delay = max_t(unsigned long, delay, HZ); to
__schedule_delayed().  Is it really necessary?

Thanks,

Ilya
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] libceph: use keepalive2 to verify the mon session is alive

[Ilya Dryomov]

On Wed, Sep 2, 2015 at 12:25 PM, Yan, Zheng  wrote:
>
>> On Sep 2, 2015, at 17:12, Ilya Dryomov  wrote:
>>
>> On Wed, Sep 2, 2015 at 5:22 AM, Yan, Zheng  wrote:
>>> timespec_to_jiffies() does not work this way. it convert time delta in form 
>>> of timespec to time delta in form of jiffies.
>>
>> Ah sorry, con->last_keepalive_ack is a realtime timespec from
>> userspace.
>>
>>>
>>> I will updated the patch according  to the rest comments .
>>
>> I still want ceph_con_keepalive_expired() to handle interval == 0
>> internally, so that opt->monc_ping_timeout can be passed without any
>> checks in the caller.
>>
>> I also noticed you added delay = max_t(unsigned long, delay, HZ); to
>> __schedule_delayed().  Is it really necessary?
>>
>
> Updated. please review.

I was concerned about user facing option names and timeouts, since
that's what I had to deal with recently - those parts look OK to me
now.

Thanks,

Ilya
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] libceph: use keepalive2 to verify the mon session is alive

[Ilya Dryomov]

On Wed, Sep 2, 2015 at 12:25 PM, Yan, Zheng  wrote:
>
>> On Sep 2, 2015, at 17:12, Ilya Dryomov  wrote:
>>
>> On Wed, Sep 2, 2015 at 5:22 AM, Yan, Zheng  wrote:
>>> timespec_to_jiffies() does not work this way. it convert time delta in form 
>>> of timespec to time delta in form of jiffies.
>>
>> Ah sorry, con->last_keepalive_ack is a realtime timespec from
>> userspace.
>>
>>>
>>> I will updated the patch according  to the rest comments .
>>
>> I still want ceph_con_keepalive_expired() to handle interval == 0
>> internally, so that opt->monc_ping_timeout can be passed without any
>> checks in the caller.
>>
>> I also noticed you added delay = max_t(unsigned long, delay, HZ); to
>> __schedule_delayed().  Is it really necessary?
>>
>
> Updated. please review.

One other thing you might want to do is to / 3 instead of >> 2 in
delayed_work().  That way the tick interval in the non-hunting case is
going to be 10 seconds, which matches the default for ping interval
(mon_client_ping_interval) in userspace.

Thanks,

Ilya
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] libceph: use keepalive2 to verify the mon session is alive

[Yan, Zheng]

> On Sep 2, 2015, at 17:12, Ilya Dryomov  wrote:
> 
> On Wed, Sep 2, 2015 at 5:22 AM, Yan, Zheng  wrote:
>> timespec_to_jiffies() does not work this way. it convert time delta in form 
>> of timespec to time delta in form of jiffies.
> 
> Ah sorry, con->last_keepalive_ack is a realtime timespec from
> userspace.
> 
>> 
>> I will updated the patch according  to the rest comments .
> 
> I still want ceph_con_keepalive_expired() to handle interval == 0
> internally, so that opt->monc_ping_timeout can be passed without any
> checks in the caller.
> 
> I also noticed you added delay = max_t(unsigned long, delay, HZ); to
> __schedule_delayed().  Is it really necessary?
> 

Updated. please review.

Regards
Yan, Zheng--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] libceph: use keepalive2 to verify the mon session is alive

[Yan, Zheng]

Signed-off-by: Yan, Zheng 
---
 include/linux/ceph/libceph.h   |  2 ++
 include/linux/ceph/messenger.h |  4 +++
 include/linux/ceph/msgr.h  |  4 ++-
 net/ceph/ceph_common.c | 18 -
 net/ceph/messenger.c   | 60 ++
 net/ceph/mon_client.c  | 38 --
 6 files changed, 111 insertions(+), 15 deletions(-)

diff --git a/include/linux/ceph/libceph.h b/include/linux/ceph/libceph.h
index 9ebee53..9a0b471 100644
--- a/include/linux/ceph/libceph.h
+++ b/include/linux/ceph/libceph.h
@@ -46,6 +46,7 @@ struct ceph_options {
unsigned long mount_timeout;/* jiffies */
unsigned long osd_idle_ttl; /* jiffies */
unsigned long osd_keepalive_timeout;/* jiffies */
+   unsigned long mon_keepalive_timeout;/* jiffies */
 
/*
 * any type that can't be simply compared or doesn't need need
@@ -66,6 +67,7 @@ struct ceph_options {
 #define CEPH_MOUNT_TIMEOUT_DEFAULT msecs_to_jiffies(60 * 1000)
 #define CEPH_OSD_KEEPALIVE_DEFAULT msecs_to_jiffies(5 * 1000)
 #define CEPH_OSD_IDLE_TTL_DEFAULT  msecs_to_jiffies(60 * 1000)
+#define CEPH_MON_KEEPALIVE_DEFAULT msecs_to_jiffies(30 * 1000)
 
 #define CEPH_MSG_MAX_FRONT_LEN (16*1024*1024)
 #define CEPH_MSG_MAX_MIDDLE_LEN(16*1024*1024)
diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph/messenger.h
index 3775327..83063b6 100644
--- a/include/linux/ceph/messenger.h
+++ b/include/linux/ceph/messenger.h
@@ -248,6 +248,8 @@ struct ceph_connection {
int in_base_pos; /* bytes read */
__le64 in_temp_ack;  /* for reading an ack */
 
+   struct timespec last_keepalive_ack;
+
struct delayed_work work;   /* send|recv work */
unsigned long   delay;  /* current delay interval */
 };
@@ -285,6 +287,8 @@ extern void ceph_msg_revoke(struct ceph_msg *msg);
 extern void ceph_msg_revoke_incoming(struct ceph_msg *msg);
 
 extern void ceph_con_keepalive(struct ceph_connection *con);
+extern int ceph_con_keepalive_expired(struct ceph_connection *con,
+ unsigned long interval);
 
 extern void ceph_msg_data_add_pages(struct ceph_msg *msg, struct page **pages,
size_t length, size_t alignment);
diff --git a/include/linux/ceph/msgr.h b/include/linux/ceph/msgr.h
index 1c18872..0fe2656 100644
--- a/include/linux/ceph/msgr.h
+++ b/include/linux/ceph/msgr.h
@@ -84,10 +84,12 @@ struct ceph_entity_inst {
 #define CEPH_MSGR_TAG_MSG   7  /* message */
 #define CEPH_MSGR_TAG_ACK   8  /* message ack */
 #define CEPH_MSGR_TAG_KEEPALIVE 9  /* just a keepalive byte! */
-#define CEPH_MSGR_TAG_BADPROTOVER  10  /* bad protocol version */
+#define CEPH_MSGR_TAG_BADPROTOVER   10 /* bad protocol version */
 #define CEPH_MSGR_TAG_BADAUTHORIZER 11 /* bad authorizer */
 #define CEPH_MSGR_TAG_FEATURES  12 /* insufficient features */
 #define CEPH_MSGR_TAG_SEQ   13 /* 64-bit int follows with seen seq 
number */
+#define CEPH_MSGR_TAG_KEEPALIVE214 /* keepalive2 byte + ceph_timespec */
+#define CEPH_MSGR_TAG_KEEPALIVE2_ACK 15 /* keepalive2 reply */
 
 
 /*
diff --git a/net/ceph/ceph_common.c b/net/ceph/ceph_common.c
index f30329f..5143f6e 100644
--- a/net/ceph/ceph_common.c
+++ b/net/ceph/ceph_common.c
@@ -226,6 +226,7 @@ static int parse_fsid(const char *str, struct ceph_fsid 
*fsid)
  * ceph options
  */
 enum {
+   Opt_monkeepalivetimeout,
Opt_osdtimeout,
Opt_osdkeepalivetimeout,
Opt_mount_timeout,
@@ -250,6 +251,7 @@ enum {
 };
 
 static match_table_t opt_tokens = {
+   {Opt_monkeepalivetimeout, "monkeepalive=%d"},
{Opt_osdtimeout, "osdtimeout=%d"},
{Opt_osdkeepalivetimeout, "osdkeepalive=%d"},
{Opt_mount_timeout, "mount_timeout=%d"},
@@ -354,9 +356,10 @@ ceph_parse_options(char *options, const char *dev_name,
 
/* start with defaults */
opt->flags = CEPH_OPT_DEFAULT;
-   opt->osd_keepalive_timeout = CEPH_OSD_KEEPALIVE_DEFAULT;
opt->mount_timeout = CEPH_MOUNT_TIMEOUT_DEFAULT;
opt->osd_idle_ttl = CEPH_OSD_IDLE_TTL_DEFAULT;
+   opt->osd_keepalive_timeout = CEPH_OSD_KEEPALIVE_DEFAULT;
+   opt->mon_keepalive_timeout = CEPH_MON_KEEPALIVE_DEFAULT;
 
/* get mon ip(s) */
/* ip1[:port1][,ip2[:port2]...] */
@@ -460,6 +463,16 @@ ceph_parse_options(char *options, const char *dev_name,
}
opt->osd_idle_ttl = msecs_to_jiffies(intval * 1000);
break;
+   case Opt_monkeepalivetimeout:
+   /* 0 isn't well defined right now, reject it */
+   if (intval < 1 || intval > INT_MAX / 1000) {
+   pr_err("monkeepalive out of range\n");
+   err = -EINVAL;
+   goto out;

Re: [PATCH] libceph: use keepalive2 to verify the mon session is alive

[Ilya Dryomov]

On Tue, Sep 1, 2015 at 5:21 PM, Yan, Zheng  wrote:
> Signed-off-by: Yan, Zheng 
> ---
>  include/linux/ceph/libceph.h   |  2 ++
>  include/linux/ceph/messenger.h |  4 +++
>  include/linux/ceph/msgr.h  |  4 ++-
>  net/ceph/ceph_common.c | 18 -
>  net/ceph/messenger.c   | 60 
> ++
>  net/ceph/mon_client.c  | 38 --
>  6 files changed, 111 insertions(+), 15 deletions(-)

Hi Zheng,

Just want to make sure you saw my "Re: " message - I had a few
suggestions.  This one looks like the one I commented on, straight from
the testing branch.

Thanks,

Ilya
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html