Hi,
Please split up the patch into parts according to functionality. I
noticed at least one "fixup" patch which converts DHCP client state
setting to use set_state(), another one for DHCP v4 support and another
for IPv6.
On Mon, 2014-01-06 at 18:09 -0800, Andrew LeCain wrote:
> dhcp/v6: On renew/rebind, send ipconfig_changed notification event
> ipconfig: Adding stats getters to allow plugins to query new lease stats
> ---
> gdhcp/client.c | 158
> +++++++++++++++++++++++++++++++++++++++++-----------
> gdhcp/gdhcp.h | 6 ++
> include/ipconfig.h | 18 ++++++
> src/connman.h | 6 ++
> src/dhcp.c | 75 +++++++++++++++++++++++++
> src/dhcpv6.c | 93 +++++++++++++++++++++++++++++++
> src/ipconfig.c | 35 ++++++++++++
> src/service.c | 6 ++
> 8 files changed, 366 insertions(+), 31 deletions(-)
>
> diff --git a/gdhcp/client.c b/gdhcp/client.c
> index b24d19d..2a74ff0 100644
> --- a/gdhcp/client.c
> +++ b/gdhcp/client.c
> @@ -94,6 +94,8 @@ struct _GDHCPClient {
> char *assigned_ip;
> time_t start;
> uint32_t lease_seconds;
> + uint32_t last_renew;
> + uint32_t last_rebind;
A renewing timer should be enough. Rebind is entered only if repeated
renewing fails, I don't think that needs extra monitoring.
After checking through the code, it seems that the thing actually needed
is the time of the next event of importance, which can be any of the
original T1, re-computed T1 if the previous one(s) fail, T2 or expiry
and so on.
> ListenMode listen_mode;
> int listener_sockfd;
> uint8_t retry_times;
> @@ -118,6 +120,8 @@ struct _GDHCPClient {
> gpointer ipv4ll_lost_data;
> GDHCPClientEventFunc address_conflict_cb;
> gpointer address_conflict_data;
> + GDHCPClientEventFunc changed_cb;
> + gpointer changed_data;
> GDHCPDebugFunc debug_func;
> gpointer debug_data;
> GDHCPClientEventFunc information_req_cb;
> @@ -170,6 +174,19 @@ static inline void debug(GDHCPClient *client, const char
> *format, ...)
> va_end(ap);
> }
>
> +static void set_state(GDHCPClient *dhcp_client, ClientState state)
> +{
> + if (state != dhcp_client->state)
> + {
> + dhcp_client->state = state;
> + debug(dhcp_client, "setting state");
If this is logged, then log the current and next states also to extract
useful information also.
> + if (dhcp_client->changed_cb!= NULL)
> + dhcp_client->changed_cb(dhcp_client,
> + dhcp_client->changed_data);
I don't see the need of waking up on every state change. There is a need
of informing the time of the next DHCP event, not every change. That
said, not all the set_state() calls below should be necessary.
How is the information made available to the plugin you're working on?
> + }
> +}
> +
> /* Initialize the packet with the proper defaults */
> static void init_packet(GDHCPClient *dhcp_client, gpointer pkt, char type)
> {
> @@ -477,6 +494,8 @@ static int send_renew(GDHCPClient *dhcp_client)
>
> add_send_options(dhcp_client, &packet);
>
> + dhcp_client->last_renew = time(NULL);
> +
> return dhcp_send_kernel_packet(&packet,
> dhcp_client->requested_ip, CLIENT_PORT,
> dhcp_client->server_ip, SERVER_PORT);
> @@ -496,6 +515,8 @@ static int send_rebound(GDHCPClient *dhcp_client)
>
> add_send_options(dhcp_client, &packet);
>
> + dhcp_client->last_rebind = time(NULL);
> +
> return dhcp_send_raw_packet(&packet, INADDR_ANY, CLIENT_PORT,
> INADDR_BROADCAST, SERVER_PORT,
> MAC_BCAST_ADDR, dhcp_client->ifindex);
> @@ -537,7 +558,7 @@ static gboolean send_probe_packet(gpointer dhcp_data)
> debug(dhcp_client, "sending IPV4LL probe request");
>
> if (dhcp_client->retry_times == 1) {
> - dhcp_client->state = IPV4LL_PROBE;
> + set_state(dhcp_client, IPV4LL_PROBE);
This set_state() change is a separate patch (part of the same patch set,
though).
> switch_listening_mode(dhcp_client, L_ARP);
> }
> ipv4ll_send_arp_packet(dhcp_client->mac_address, 0,
> @@ -1389,7 +1410,7 @@ static void ipv4ll_stop(GDHCPClient *dhcp_client)
> dhcp_client->listener_watch = 0;
> }
>
> - dhcp_client->state = IPV4LL_PROBE;
> + set_state(dhcp_client, IPV4LL_PROBE);
> dhcp_client->retry_times = 0;
> dhcp_client->requested_ip = 0;
>
> @@ -1431,7 +1452,7 @@ static int ipv4ll_recv_arp_packet(GDHCPClient
> *dhcp_client)
> if (dhcp_client->state == IPV4LL_MONITOR) {
> if (!source_conflict)
> return 0;
> - dhcp_client->state = IPV4LL_DEFEND;
> + set_state(dhcp_client, IPV4LL_DEFEND);
> debug(dhcp_client, "DEFEND mode conflicts : %d",
> dhcp_client->conflicts);
> /*Try to defend with a single announce*/
> @@ -1598,7 +1619,7 @@ static void start_request(GDHCPClient *dhcp_client)
> }
>
> if (dhcp_client->retry_times == 0) {
> - dhcp_client->state = REQUESTING;
> + set_state(dhcp_client, REQUESTING);
> switch_listening_mode(dhcp_client, L2);
> }
>
> @@ -1640,7 +1661,7 @@ static void restart_dhcp(GDHCPClient *dhcp_client, int
> retry_times)
>
> dhcp_client->retry_times = retry_times;
> dhcp_client->requested_ip = 0;
> - dhcp_client->state = INIT_SELECTING;
> + set_state(dhcp_client, INIT_SELECTING);
> switch_listening_mode(dhcp_client, L2);
>
> g_dhcp_client_start(dhcp_client, dhcp_client->last_address);
> @@ -1683,7 +1704,7 @@ static void start_rebound(GDHCPClient *dhcp_client)
> {
> debug(dhcp_client, "start rebound");
>
> - dhcp_client->state = REBINDING;
> + set_state(dhcp_client, REBINDING);
>
> dhcp_client->timeout = g_timeout_add_seconds_full(G_PRIORITY_HIGH,
> dhcp_client->lease_seconds >> 1,
> @@ -1711,7 +1732,7 @@ static gboolean start_renew_timeout(gpointer user_data)
>
> debug(dhcp_client, "start renew timeout");
>
> - dhcp_client->state = RENEWING;
> + set_state(dhcp_client, RENEWING);
>
> dhcp_client->lease_seconds >>= 1;
>
> @@ -1739,7 +1760,8 @@ static void start_bound(GDHCPClient *dhcp_client)
> {
> debug(dhcp_client, "start bound");
>
> - dhcp_client->state = BOUND;
> + set_state(dhcp_client, BOUND);
> +
Unnecessary newline.
> if (dhcp_client->timeout > 0)
> g_source_remove(dhcp_client->timeout);
> @@ -2325,13 +2347,14 @@ static gboolean listener_event(GIOChannel *channel,
> GIOCondition condition,
> dhcp_client->server_ip = get_be32(option);
> dhcp_client->requested_ip = ntohl(packet.yiaddr);
>
> - dhcp_client->state = REQUESTING;
> + set_state(dhcp_client, REQUESTING);
>
> start_request(dhcp_client);
>
> return TRUE;
> case REQUESTING:
> case RENEWING:
> + dhcp_client->last_renew = time(NULL);
Shouldn't the time for the upcoming renew be remembered instead of the
current time?
> case REBINDING:
> if (*message_type == DHCPACK) {
> dhcp_client->retry_times = 0;
> @@ -2342,6 +2365,10 @@ static gboolean listener_event(GIOChannel *channel,
> GIOCondition condition,
>
> dhcp_client->lease_seconds = get_lease(&packet);
>
> + dhcp_client->expire = time(NULL) +
> dhcp_client->lease_seconds;
> +
> + dhcp_client->last_rebind = time(NULL);
> +
> get_request(dhcp_client, &packet);
>
> switch_listening_mode(dhcp_client, L_NONE);
> @@ -2572,7 +2599,7 @@ static gboolean ipv4ll_defend_timeout(gpointer
> dhcp_data)
> debug(dhcp_client, "back to MONITOR mode");
>
> dhcp_client->conflicts = 0;
> - dhcp_client->state = IPV4LL_MONITOR;
> + set_state(dhcp_client, IPV4LL_MONITOR);
>
> return FALSE;
> }
> @@ -2593,7 +2620,7 @@ static gboolean ipv4ll_announce_timeout(gpointer
> dhcp_data)
>
> ip = htonl(dhcp_client->requested_ip);
> debug(dhcp_client, "switching to monitor mode");
> - dhcp_client->state = IPV4LL_MONITOR;
> + set_state(dhcp_client, IPV4LL_MONITOR);
> dhcp_client->assigned_ip = get_ip(ip);
>
> if (dhcp_client->ipv4ll_available_cb)
> @@ -2613,7 +2640,7 @@ static gboolean ipv4ll_probe_timeout(gpointer dhcp_data)
> dhcp_client->retry_times);
>
> if (dhcp_client->retry_times == PROBE_NUM) {
> - dhcp_client->state = IPV4LL_ANNOUNCE;
> + set_state(dhcp_client, IPV4LL_ANNOUNCE);
> dhcp_client->retry_times = 0;
>
> dhcp_client->retry_times++;
> @@ -2633,80 +2660,80 @@ int g_dhcp_client_start(GDHCPClient *dhcp_client,
> const char *last_address)
>
> if (dhcp_client->type == G_DHCP_IPV6) {
> if (dhcp_client->information_req_cb) {
> - dhcp_client->state = INFORMATION_REQ;
> + set_state(dhcp_client, INFORMATION_REQ);
Indent with tabs here and in all the other places also, please.
> re = switch_listening_mode(dhcp_client, L3);
> if (re != 0) {
> switch_listening_mode(dhcp_client, L_NONE);
> - dhcp_client->state = 0;
> + set_state(dhcp_client, 0);
> return re;
> }
> send_information_req(dhcp_client);
>
> } else if (dhcp_client->solicitation_cb) {
> - dhcp_client->state = SOLICITATION;
> + set_state(dhcp_client, SOLICITATION);
> re = switch_listening_mode(dhcp_client, L3);
> if (re != 0) {
> switch_listening_mode(dhcp_client, L_NONE);
> - dhcp_client->state = 0;
> + set_state(dhcp_client, 0);
> return re;
> }
> send_solicitation(dhcp_client);
>
> } else if (dhcp_client->request_cb) {
> - dhcp_client->state = REQUEST;
> + set_state(dhcp_client, REQUEST);
> re = switch_listening_mode(dhcp_client, L3);
> if (re != 0) {
> switch_listening_mode(dhcp_client, L_NONE);
> - dhcp_client->state = 0;
> + set_state(dhcp_client, 0);
> return re;
> }
> send_dhcpv6_request(dhcp_client);
>
> } else if (dhcp_client->confirm_cb) {
> - dhcp_client->state = CONFIRM;
> + set_state(dhcp_client, CONFIRM);
> re = switch_listening_mode(dhcp_client, L3);
> if (re != 0) {
> switch_listening_mode(dhcp_client, L_NONE);
> - dhcp_client->state = 0;
> + set_state(dhcp_client, 0);
> return re;
> }
> send_dhcpv6_confirm(dhcp_client);
>
> } else if (dhcp_client->renew_cb) {
> - dhcp_client->state = RENEW;
> + set_state(dhcp_client, RENEW);
> re = switch_listening_mode(dhcp_client, L3);
> if (re != 0) {
> switch_listening_mode(dhcp_client, L_NONE);
> - dhcp_client->state = 0;
> + set_state(dhcp_client, 0);
> return re;
> }
> send_dhcpv6_renew(dhcp_client);
>
> } else if (dhcp_client->rebind_cb) {
> - dhcp_client->state = REBIND;
> + set_state(dhcp_client, REBIND);
> re = switch_listening_mode(dhcp_client, L3);
> if (re != 0) {
> switch_listening_mode(dhcp_client, L_NONE);
> - dhcp_client->state = 0;
> + set_state(dhcp_client, 0);
> return re;
> }
> send_dhcpv6_rebind(dhcp_client);
>
> } else if (dhcp_client->release_cb) {
> - dhcp_client->state = RENEW;
> + set_state(dhcp_client, RENEW);
> re = switch_listening_mode(dhcp_client, L3);
> if (re != 0) {
> switch_listening_mode(dhcp_client, L_NONE);
> - dhcp_client->state = 0;
> + set_state(dhcp_client, 0);
> return re;
> }
> send_dhcpv6_release(dhcp_client);
> } else if (dhcp_client->decline_cb) {
> - dhcp_client->state = DECLINE;
> + set_state(dhcp_client, DECLINE);
> re = switch_listening_mode(dhcp_client, L3);
> if (re != 0) {
> switch_listening_mode(dhcp_client, L_NONE);
> - dhcp_client->state = 0;
> + set_state(dhcp_client, 0);
> return re;
> }
> send_dhcpv6_decline(dhcp_client);
> @@ -2716,7 +2743,7 @@ int g_dhcp_client_start(GDHCPClient *dhcp_client, const
> char *last_address)
> }
>
> if (dhcp_client->type == G_DHCP_IPV4LL) {
> - dhcp_client->state = INIT_SELECTING;
> + set_state(dhcp_client, INIT_SELECTING);
> ipv4ll_start(dhcp_client);
> return 0;
> }
> @@ -2733,7 +2760,7 @@ int g_dhcp_client_start(GDHCPClient *dhcp_client, const
> char *last_address)
> g_free(dhcp_client->assigned_ip);
> dhcp_client->assigned_ip = NULL;
>
> - dhcp_client->state = INIT_SELECTING;
> + set_state(dhcp_client, INIT_SELECTING);
> re = switch_listening_mode(dhcp_client, L2);
> if (re != 0)
> return re;
> @@ -2789,7 +2816,7 @@ void g_dhcp_client_stop(GDHCPClient *dhcp_client)
> dhcp_client->ack_retry_times = 0;
>
> dhcp_client->requested_ip = 0;
> - dhcp_client->state = RELEASED;
> + set_state(dhcp_client, RELEASED);
> dhcp_client->lease_seconds = 0;
> }
>
> @@ -2834,6 +2861,10 @@ void g_dhcp_client_register_event(GDHCPClient
> *dhcp_client,
> dhcp_client->address_conflict_cb = func;
> dhcp_client->address_conflict_data = data;
> return;
> + case G_DHCP_CLIENT_EVENT_CHANGED:
> + dhcp_client->changed_cb = func;
> + dhcp_client->changed_data = data;
> + return;
> case G_DHCP_CLIENT_EVENT_INFORMATION_REQ:
> if (dhcp_client->type != G_DHCP_IPV6)
> return;
> @@ -2936,6 +2967,71 @@ char *g_dhcp_client_get_netmask(GDHCPClient
> *dhcp_client)
> return NULL;
> }
>
> +int g_dhcp_client_get_timeouts(GDHCPClient *dhcp_client,
> + uint32_t *lease_seconds, time_t *last_renew,
> + time_t *last_rebind, time_t *expire)
> +{
> +
> + if (dhcp_client == NULL || dhcp_client->type == G_DHCP_IPV6)
> + return -EINVAL;
> +
> + if (lease_seconds != NULL)
> + *lease_seconds = dhcp_client->lease_seconds;
> +
> + if (last_renew != NULL)
> + *last_renew = dhcp_client->last_renew;
> +
> + if (last_rebind != NULL)
> + *last_rebind = dhcp_client->last_rebind;
> +
> + if (expire != NULL)
> + *expire = dhcp_client->expire;
> +
> + return 0;
I don't see how we get any future information from this. last_renew
points to the time when the renew was issued, not when it is going to
expire. Wasn't the intention of all this to be able to sleep until the
next DHCP event (i.e. Renew) is about to happen?
The Renew timeout is computed in start_bound(), so we should fish it out
from there instead. BTW, start_bound() is somewhat bogus and should be
fixed. Yes, the default T1 is half of the time until expiry, but T1 and
T2 can be set by the server to something else. In order to function
correctly, the usage of T1 and T2 should also be fixed (that's a totally
different story), and with that the value must be taken from the one
computed in start_bound() as it eventually will be working properly.
I don't anything else except the time for the next event is needed, be
it caused by renew, rebind or expire. The details are not interesting
when the intention is to inform how long one can idle.
> +
> +
> +}
> +
> +const char* g_dhcp_client_get_state(GDHCPClient *dhcp_client)
> +{
> + switch (dhcp_client->state) {
> + case INIT_SELECTING:
> + return "INIT_SELECTING";
> + case REQUESTING:
> + return "REQUESTING";
> + case BOUND:
> + return "BOUND";
> + case RENEWING:
> + return "RENEWING";
> + case REBINDING:
> + return "REBINDING";
> + case RELEASED:
> + return "RELEASED";
> + case IPV4LL_PROBE:
> + return "IPV4LL_PROBE";
> + case IPV4LL_ANNOUNCE:
> + return "IPV4LL_ANNOUNCE";
> + case IPV4LL_MONITOR:
> + return "IPV4LL_MONITOR";
> + case IPV4LL_DEFEND:
> + return "IPV4LL_DEFEND";
> + case INFORMATION_REQ:
> + return "INFORMATION_REQ";
> + case SOLICITATION:
> + return "SOLICITATION";
> + case REQUEST:
> + return "REQUEST";
> + case RENEW:
> + return "RENEW";
> + case REBIND:
> + return "REBIND";
> + case RELEASE:
> + return "RELEASE";
> + default:
> + return NULL;
> + }
> +
> +}
> GDHCPClientError g_dhcp_client_set_request(GDHCPClient *dhcp_client,
> unsigned int option_code)
> {
> diff --git a/gdhcp/gdhcp.h b/gdhcp/gdhcp.h
> index 0d79361..84286a8 100644
> --- a/gdhcp/gdhcp.h
> +++ b/gdhcp/gdhcp.h
> @@ -61,6 +61,7 @@ typedef enum {
> G_DHCP_CLIENT_EVENT_RENEW,
> G_DHCP_CLIENT_EVENT_REBIND,
> G_DHCP_CLIENT_EVENT_RELEASE,
> + G_DHCP_CLIENT_EVENT_CHANGED,
> G_DHCP_CLIENT_EVENT_CONFIRM,
> G_DHCP_CLIENT_EVENT_DECLINE,
> } GDHCPClientEvent;
> @@ -155,6 +156,11 @@ char *g_dhcp_client_get_netmask(GDHCPClient *client);
> GList *g_dhcp_client_get_option(GDHCPClient *client,
> unsigned char option_code);
> int g_dhcp_client_get_index(GDHCPClient *client);
> +int g_dhcp_client_get_timeouts(GDHCPClient *dhcp_client,
> + uint32_t *lease_seconds, time_t *last_renew,
> + time_t *last_rebind, time_t *expire);
> +
> +const char* g_dhcp_client_get_state(GDHCPClient *state);
>
> void g_dhcp_client_set_debug(GDHCPClient *client,
> GDHCPDebugFunc func, gpointer user_data);
> diff --git a/include/ipconfig.h b/include/ipconfig.h
> index a86b295..0a8357c 100644
> --- a/include/ipconfig.h
> +++ b/include/ipconfig.h
> @@ -23,6 +23,8 @@
> #define __CONNMAN_IPCONFIG_H
>
> #include <connman/ipaddress.h>
> +#include <time.h>
> +#include <stdint.h>
>
> #ifdef __cplusplus
> extern "C" {
> @@ -51,6 +53,22 @@ enum connman_ipconfig_method {
> };
>
> struct connman_ipconfig;
> +struct connman_dhcp;
> +struct connman_dhcpv6;
> +
> +struct dhcp_stats{
> + char* state;
> + time_t lease_time;
> + uint32_t lease_seconds;
> + time_t last_renew;
> + time_t last_rebind;
> + time_t expire;
> +};
No, we don't want the stats. We want to know when the next event of some
importance happens. For that one a simple time value in the future is
enough.
> +enum connman_ipconfig_method connman_ipconfig_get_ipconfig_method(struct
> connman_ipconfig *ipconfig);
> +enum connman_ipconfig_type connman_ipconfig_get_ipconfig_type(struct
> connman_ipconfig *ipconfig);
> +struct dhcp_stats* connman_ipconfig_get_dhcp_stats(struct connman_ipconfig
> *ipconfig);
> +struct dhcp_stats* connman_ipconfig_get_dhcpv6_stats(struct connman_ipconfig
> *ipconfig);
>
> #ifdef __cplusplus
> }
> diff --git a/src/connman.h b/src/connman.h
> index bf59dbf..c1e7962 100644
> --- a/src/connman.h
> +++ b/src/connman.h
> @@ -393,6 +393,9 @@ void __connman_ipconfig_set_dhcpv6_prefixes(struct
> connman_ipconfig *ipconfig,
> char **prefixes);
> char **__connman_ipconfig_get_dhcpv6_prefixes(struct connman_ipconfig
> *ipconfig);
>
> +void __connman_ipconfig_set_dhcp(struct connman_ipconfig *ipconfig,
> + void *dhcp);
> +
> int __connman_ipconfig_load(struct connman_ipconfig *ipconfig,
> GKeyFile *keyfile, const char *identifier, const char *prefix);
> int __connman_ipconfig_save(struct connman_ipconfig *ipconfig,
> @@ -438,6 +441,8 @@ int __connman_dhcp_start(struct connman_network *network,
> dhcp_cb callback);
> void __connman_dhcp_stop(struct connman_network *network);
> int __connman_dhcp_init(void);
> void __connman_dhcp_cleanup(void);
> +struct dhcp_stats* __connman_dhcp_get_stats(struct connman_dhcp *dhcp);
> +struct dhcp_stats* __connman_dhcpv6_get_stats(struct connman_dhcpv6 *dhcp);
> int __connman_dhcpv6_init(void);
> void __connman_dhcpv6_cleanup(void);
> int __connman_dhcpv6_start_info(struct connman_network *network,
> @@ -710,6 +715,7 @@ void __connman_service_return_error(struct
> connman_service *service,
> void __connman_service_reply_dbus_pending(DBusMessage *pending, int error,
> const char *path);
>
> +void __connman_service_ipconfig_changed(struct connman_service *service,
> struct connman_ipconfig *ipconfig);
> int __connman_service_provision_changed(const char *ident);
> void __connman_service_set_config(struct connman_service *service,
> const char *file_id, const char *section);
> diff --git a/src/dhcp.c b/src/dhcp.c
> index 2193e4d..1ab8fc9 100644
> --- a/src/dhcp.c
> +++ b/src/dhcp.c
> @@ -50,6 +50,7 @@ struct connman_dhcp {
>
> GDHCPClient *ipv4ll_client;
> GDHCPClient *dhcp_client;
> + struct dhcp_stats stats;
> char *ipv4ll_debug_prefix;
> char *dhcp_debug_prefix;
> };
> @@ -120,6 +121,14 @@ static void dhcp_invalidate(struct connman_dhcp *dhcp,
> bool callback)
>
> __connman_ipconfig_set_dhcp_address(ipconfig,
> __connman_ipconfig_get_local(ipconfig));
> + __connman_ipconfig_set_dhcp(ipconfig, NULL);
> +
> + dhcp->stats.lease_time = 0;
> + dhcp->stats.lease_seconds = 0;
> + dhcp->stats.last_renew = 0;
> + dhcp->stats.last_rebind = 0;
> + dhcp->stats.expire = 0;
> +
> DBG("last address %s", __connman_ipconfig_get_dhcp_address(ipconfig));
>
> __connman_ipconfig_address_remove(ipconfig);
> @@ -297,6 +306,53 @@ static bool compare_string_arrays(char **array_a, char
> **array_b)
> return true;
> }
>
> +static void lease_renew_cb(GDHCPClient *dhcp_client, gpointer user_data)
> +{
> + struct connman_dhcp *dhcp = user_data;
> + struct connman_service *service;
> + struct connman_ipconfig *ipconfig;
> +
> + //copy info into local storage
> + g_dhcp_client_get_timeouts(dhcp_client,&(dhcp->stats.lease_seconds),
> +
> &(dhcp->stats.last_renew),&(dhcp->stats.last_rebind),&(dhcp->stats.expire));
> + //inform service about the change
> + service = connman_service_lookup_from_network(dhcp->network);
> + if (service == NULL)
> + return;
> +
> + ipconfig = __connman_service_get_ip4config(service);
> + if (ipconfig == NULL)
> + return;
> + __connman_service_ipconfig_changed(service,ipconfig);
> +}
> +
> +static void dhcp_state_cb(GDHCPClient *dhcp_client, gpointer user_data)
> +{
> +
> + struct connman_dhcp *dhcp = user_data;
> + struct connman_service *service;
> + struct connman_ipconfig *ipconfig;
> + const char *state;
> +
> + state = g_dhcp_client_get_state(dhcp_client);
> +
> + g_free(dhcp->stats.state);
> + dhcp->stats.state = g_strdup(state);
> +
> + //inform service about the change
> + service = connman_service_lookup_from_network(dhcp->network);
> + if (service == NULL)
> + return;
> +
> + ipconfig = __connman_service_get_ip4config(service);
> +
> + if (ipconfig == NULL)
> + return;
> +
> + __connman_service_ipconfig_changed(service,ipconfig);
> +
> +}
> +
> static void lease_available_cb(GDHCPClient *dhcp_client, gpointer user_data)
> {
> struct connman_dhcp *dhcp = user_data;
> @@ -335,10 +391,15 @@ static void lease_available_cb(GDHCPClient
> *dhcp_client, gpointer user_data)
> c_prefixlen = __connman_ipconfig_get_prefixlen(ipconfig);
>
> address = g_dhcp_client_get_address(dhcp_client);
> + dhcp->stats.lease_time = time(NULL);
>
> __connman_ipconfig_set_dhcp_address(ipconfig, address);
> + __connman_ipconfig_set_dhcp(ipconfig, dhcp);
> DBG("last address %s", address);
>
> +
> g_dhcp_client_get_timeouts(dhcp_client,&(dhcp->stats.lease_seconds),&(dhcp->stats.last_renew),
> + &(dhcp->stats.last_rebind),&(dhcp->stats.expire));
> +
> option = g_dhcp_client_get_option(dhcp_client, G_DHCP_SUBNET);
> if (option)
> netmask = g_strdup(option->data);
> @@ -543,11 +604,20 @@ static int dhcp_request(struct connman_dhcp *dhcp)
> lease_available_cb, dhcp);
>
> g_dhcp_client_register_event(dhcp_client,
> + G_DHCP_CLIENT_EVENT_RENEW, lease_renew_cb, dhcp);
> +
> + g_dhcp_client_register_event(dhcp_client,
> G_DHCP_CLIENT_EVENT_LEASE_LOST, lease_lost_cb, dhcp);
>
> g_dhcp_client_register_event(dhcp_client,
> G_DHCP_CLIENT_EVENT_NO_LEASE, no_lease_cb, dhcp);
>
> + g_dhcp_client_register_event(dhcp_client,
> + G_DHCP_CLIENT_EVENT_CHANGED, dhcp_state_cb, dhcp);
> +
> + g_dhcp_client_register_event(dhcp_client,
> + G_DHCP_CLIENT_EVENT_CHANGED, dhcp_state_cb, dhcp);
> +
> dhcp->dhcp_client = dhcp_client;
>
> ipconfig = __connman_service_get_ip4config(service);
> @@ -644,3 +714,8 @@ void __connman_dhcp_cleanup(void)
> g_hash_table_destroy(network_table);
> network_table = NULL;
> }
> +
> +struct dhcp_stats* __connman_dhcp_get_stats(struct connman_dhcp *instance)
> +{
> + return &(instance->stats);
> +}
> diff --git a/src/dhcpv6.c b/src/dhcpv6.c
> index 789db10..23afdf4 100644
> --- a/src/dhcpv6.c
> +++ b/src/dhcpv6.c
> @@ -81,6 +81,7 @@ struct connman_dhcpv6 {
> int request_count; /* how many times REQUEST have been sent */
> bool stateless; /* TRUE if stateless DHCPv6 is used */
> bool started; /* TRUE if we have DHCPv6 started */
> + struct dhcp_stats stats;
> };
>
> static GHashTable *network_table;
> @@ -283,8 +284,38 @@ static void clear_callbacks(GDHCPClient *dhcp_client)
> g_dhcp_client_register_event(dhcp_client,
> G_DHCP_CLIENT_EVENT_INFORMATION_REQ,
> NULL, NULL);
> +
> + g_dhcp_client_register_event(dhcp_client,
> + G_DHCP_CLIENT_EVENT_CHANGED,
> + NULL, NULL);
> }
>
> +static void dhcpv6_state_cb(GDHCPClient *dhcp_client, gpointer user_data)
> +{
> +
> + struct connman_dhcpv6 *dhcp = user_data;
> + struct connman_service *service;
> + struct connman_ipconfig *ipconfig;
> + const char *state;
> +
> + state = g_dhcp_client_get_state(dhcp_client);
> +
> + g_free(dhcp->stats.state);
> + dhcp->stats.state = g_strdup(state);
> +
> + //inform service about the change
> + service = connman_service_lookup_from_network(dhcp->network);
> + if (service == NULL)
> + return;
> +
> + ipconfig = __connman_service_get_ip6config(service);
> +
> + if (ipconfig == NULL)
> + return;
> +
> + __connman_service_ipconfig_changed(service,ipconfig);
> +
> +}
> static void info_req_cb(GDHCPClient *dhcp_client, gpointer user_data)
> {
> struct connman_dhcpv6 *dhcp = user_data;
> @@ -413,6 +444,12 @@ static int dhcpv6_info_request(struct connman_dhcpv6
> *dhcp)
> g_dhcp_client_register_event(dhcp_client,
> G_DHCP_CLIENT_EVENT_INFORMATION_REQ, info_req_cb, dhcp);
>
> + g_dhcp_client_register_event(dhcp_client,
> + G_DHCP_CLIENT_EVENT_CHANGED, dhcpv6_state_cb, dhcp);
> +
> + g_dhcp_client_register_event(dhcp_client,
> + G_DHCP_CLIENT_EVENT_CHANGED, dhcpv6_state_cb, dhcp);
> +
> dhcp->dhcp_client = dhcp_client;
>
> return g_dhcp_client_start(dhcp_client, NULL);
> @@ -1035,6 +1072,8 @@ static void re_cb(enum request_type req_type,
> GDHCPClient *dhcp_client,
> {
> struct connman_dhcpv6 *dhcp = user_data;
> uint16_t status;
> + time_t expired;
> + time_t lease_time;
>
> clear_timer(dhcp);
>
> @@ -1042,6 +1081,17 @@ static void re_cb(enum request_type req_type,
> GDHCPClient *dhcp_client,
>
> DBG("dhcpv6 cb msg %p status %d", dhcp, status);
>
> + g_dhcpv6_client_get_timeouts(dhcp->dhcp_client, NULL, NULL,
> + &lease_time, &expired);
> +
> + //write stats out
If one must have comments, use /* */. The below is obvious enought
without comments.
> +
> + dhcp->stats.lease_time = lease_time;
> + dhcp->stats.lease_seconds = expired - lease_time;
> + //dhcp->stats.last_renew = last_renew;
> + //dhcp->stats.last_rebind = last_rebind;
Please don't add commented out code.
> + dhcp->stats.expire = expired;
> +
> /*
> * RFC 3315, 18.1.8 handle the resend if error
> */
> @@ -1151,6 +1201,12 @@ static int dhcpv6_rebind(struct connman_dhcpv6 *dhcp)
> g_dhcp_client_register_event(dhcp_client, G_DHCP_CLIENT_EVENT_REBIND,
> rebind_cb, dhcp);
>
> + g_dhcp_client_register_event(dhcp_client,
> + G_DHCP_CLIENT_EVENT_CHANGED, dhcpv6_state_cb, dhcp);
> +
> + g_dhcp_client_register_event(dhcp_client,
> + G_DHCP_CLIENT_EVENT_CHANGED, dhcpv6_state_cb, dhcp);
> +
> dhcp->dhcp_client = dhcp_client;
>
> return g_dhcp_client_start(dhcp_client, NULL);
> @@ -1270,6 +1326,12 @@ static int dhcpv6_request(struct connman_dhcpv6 *dhcp,
> g_dhcp_client_register_event(dhcp_client, G_DHCP_CLIENT_EVENT_REQUEST,
> request_cb, dhcp);
>
> + g_dhcp_client_register_event(dhcp_client,
> + G_DHCP_CLIENT_EVENT_CHANGED, dhcpv6_state_cb, dhcp);
> +
> + g_dhcp_client_register_event(dhcp_client,
> + G_DHCP_CLIENT_EVENT_CHANGED, dhcpv6_state_cb, dhcp);
> +
> dhcp->dhcp_client = dhcp_client;
>
> return g_dhcp_client_start(dhcp_client, NULL);
> @@ -1341,6 +1403,12 @@ static int dhcpv6_renew(struct connman_dhcpv6 *dhcp)
> g_dhcp_client_register_event(dhcp_client, G_DHCP_CLIENT_EVENT_RENEW,
> renew_cb, dhcp);
>
> + g_dhcp_client_register_event(dhcp_client,
> + G_DHCP_CLIENT_EVENT_CHANGED, dhcpv6_state_cb, dhcp);
> +
> + g_dhcp_client_register_event(dhcp_client,
> + G_DHCP_CLIENT_EVENT_CHANGED, dhcpv6_state_cb, dhcp);
> +
> dhcp->dhcp_client = dhcp_client;
>
> return g_dhcp_client_start(dhcp_client, NULL);
> @@ -1744,6 +1812,12 @@ static int dhcpv6_solicitation(struct connman_dhcpv6
> *dhcp)
> G_DHCP_CLIENT_EVENT_ADVERTISE,
> advertise_cb, dhcp);
>
> + g_dhcp_client_register_event(dhcp_client,
> + G_DHCP_CLIENT_EVENT_CHANGED, dhcpv6_state_cb, dhcp);
> +
> + g_dhcp_client_register_event(dhcp_client,
> + G_DHCP_CLIENT_EVENT_CHANGED, dhcpv6_state_cb, dhcp);
> +
> dhcp->dhcp_client = dhcp_client;
>
> return g_dhcp_client_start(dhcp_client, NULL);
> @@ -1922,6 +1996,13 @@ int __connman_dhcpv6_start(struct connman_network
> *network,
> if (!dhcp)
> return -ENOMEM;
>
> + service = connman_service_lookup_from_network(dhcp->network);
> + if (service == NULL)
> + return -1;
> +
> + ipconfig_ipv6 = __connman_service_get_ip6config(service);
> + __connman_ipconfig_set_dhcp(ipconfig_ipv6, dhcp);
> +
> dhcp->network = network;
> dhcp->callback = callback;
> dhcp->prefixes = prefixes;
> @@ -2680,6 +2761,7 @@ int __connman_dhcpv6_start_pd(int index, GSList
> *prefixes, dhcpv6_cb callback)
> struct connman_service *service;
> struct connman_network *network;
> struct connman_dhcpv6 *dhcp;
> + struct connman_ipconfig *ipconfig_ipv6;
>
> if (index < 0)
> return 0;
> @@ -2704,6 +2786,13 @@ int __connman_dhcpv6_start_pd(int index, GSList
> *prefixes, dhcpv6_cb callback)
> if (!dhcp)
> return -ENOMEM;
>
> + service = connman_service_lookup_from_network(dhcp->network);
> + if (service == NULL)
> + return -1;
> +
> + ipconfig_ipv6 = __connman_service_get_ip6config(service);
> + __connman_ipconfig_set_dhcp(ipconfig_ipv6, dhcp);
> +
> dhcp->network = network;
> dhcp->callback = callback;
> dhcp->started = true;
> @@ -2794,3 +2883,7 @@ void __connman_dhcpv6_cleanup(void)
> g_hash_table_destroy(network_pd_table);
> network_pd_table = NULL;
> }
> +struct dhcp_stats* __connman_dhcpv6_get_stats(struct connman_dhcpv6
> *instance)
> +{
> + return &(instance->stats);
> +}
> diff --git a/src/ipconfig.c b/src/ipconfig.c
> index 9452125..86bce56 100644
> --- a/src/ipconfig.c
> +++ b/src/ipconfig.c
> @@ -57,6 +57,7 @@ struct connman_ipconfig {
>
> int ipv6_privacy_config;
> char *last_dhcp_address;
> + void *dhcp;
> char **last_dhcpv6_prefixes;
> };
>
> @@ -1478,6 +1479,16 @@ void __connman_ipconfig_set_dhcp_address(struct
> connman_ipconfig *ipconfig,
> ipconfig->last_dhcp_address = g_strdup(address);
> }
>
> +void __connman_ipconfig_set_dhcp(struct connman_ipconfig *ipconfig,
> + void *dhcp)
> +{
> + if (ipconfig == NULL)
> + return;
> +
> + ipconfig->dhcp= dhcp;
> +
> +}
> +
> char *__connman_ipconfig_get_dhcp_address(struct connman_ipconfig *ipconfig)
> {
> if (!ipconfig)
> @@ -2395,3 +2406,27 @@ void __connman_ipconfig_cleanup(void)
> g_hash_table_destroy(ipdevice_hash);
> ipdevice_hash = NULL;
> }
> +
> +enum connman_ipconfig_method connman_ipconfig_get_ipconfig_method(struct
> connman_ipconfig *ipconfig)
> +{
> + return ipconfig->method;
> +
> +}
> +enum connman_ipconfig_type connman_ipconfig_get_ipconfig_type(struct
> connman_ipconfig *ipconfig)
> +{
> + return ipconfig->type;
> +}
> +struct dhcp_stats* connman_ipconfig_get_dhcp_stats(struct connman_ipconfig
> *ipconfig)
> +{
> + if(ipconfig->dhcp){
> + return __connman_dhcp_get_stats(ipconfig->dhcp);
> + }
> + return NULL;
> +}
> +struct dhcp_stats* connman_ipconfig_get_dhcpv6_stats(struct connman_ipconfig
> *ipconfig)
> +{
> + if(ipconfig->dhcp){
> + return __connman_dhcpv6_get_stats(ipconfig->dhcp);
> + }
> + return NULL;
> +}
> diff --git a/src/service.c b/src/service.c
> index 33cce14..ecb3388 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -1842,6 +1842,12 @@ static void settings_changed(struct connman_service
> *service,
> __connman_notifier_ipconfig_changed(service, ipconfig);
> }
>
> +void __connman_service_ipconfig_changed(struct connman_service *service,
> + struct connman_ipconfig *ipconfig)
Since IP configuration has not changed, this function is badly named.
> +{
> + settings_changed(service, ipconfig);
No IP changes here, so let's not send out any extra D-Bus messages.
> +}
> +
> static void ipv4_configuration_changed(struct connman_service *service)
> {
> if (!allow_property_changed(service))
Cheers,
Patrik
_______________________________________________
connman mailing list
[email protected]
https://lists.connman.net/mailman/listinfo/connman
