Re: [PATCH IPv6] Separate IPv4 and IPv6 in struct connman_ipconfig so that connman can work with IPv6 only networks.

2010-11-30 Thread Samuel Ortiz
Hi Jukka,

My comments below:

On Mon, Nov 22, 2010 at 06:11:32PM +0200, Jukka Rissanen wrote:
 diff --git a/src/ipconfig.c b/src/ipconfig.c
 index a872dff..2e4a910 100644
 --- a/src/ipconfig.c
 +++ b/src/ipconfig.c
 @@ -50,8 +50,6 @@ struct connman_ipconfig {
   enum connman_ipconfig_method method;
   struct connman_ipaddress *address;
   struct connman_ipaddress *system;
 -
 - struct connman_ipconfig *ipv6;
  };
  
  struct connman_ipdevice {
 @@ -76,10 +74,14 @@ struct connman_ipdevice {
  
   char *pac;
  
 - struct connman_ipconfig *config;
 + struct connman_ipconfig *config_ipv4;
 + struct connman_ipconfig *config_ipv6;
 +
 + struct connman_ipconfig_driver *driver_ipv4;
 + struct connman_ipconfig *driver_config_ipv4;
  
 - struct connman_ipconfig_driver *driver;
 - struct connman_ipconfig *driver_config;
 + struct connman_ipconfig_driver *driver_ipv6;
 + struct connman_ipconfig *driver_config_ipv6;
So the whole connman_ipconfig_driver stuff is not used anywhere, and I'm
planning to remove it sooner than later. So the chunks here and below would
probably become irrelevant.
You should not care about it for now, just a heads up.


 @@ -431,7 +501,7 @@ static void update_stats(struct connman_ipdevice 
 *ipdevice,
   connman_info(%s {TX} %u packets %u bytes, ipdevice-ifname,
   stats-tx_packets, stats-tx_bytes);
  
 - if (ipdevice-config == NULL)
 + if (ipdevice-config_ipv4 == NULL  ipdevice-config_ipv6 == NULL)
   return;
  
   ipdevice-rx_packets = stats-rx_packets;
 @@ -443,11 +513,18 @@ static void update_stats(struct connman_ipdevice 
 *ipdevice,
   ipdevice-rx_dropped = stats-rx_dropped;
   ipdevice-tx_dropped = stats-tx_dropped;
  
 - __connman_service_notify(ipdevice-config,
 - ipdevice-rx_packets, ipdevice-tx_packets,
 - ipdevice-rx_bytes, ipdevice-tx_bytes,
 - ipdevice-rx_errors, ipdevice-tx_errors,
 - ipdevice-rx_dropped, ipdevice-tx_dropped);
 + if (ipdevice-config_ipv4)
 + __connman_service_notify(ipdevice-config_ipv4,
 + ipdevice-rx_packets, 
 ipdevice-tx_packets,
 + ipdevice-rx_bytes, ipdevice-tx_bytes,
 + ipdevice-rx_errors, 
 ipdevice-tx_errors,
 + ipdevice-rx_dropped, 
 ipdevice-tx_dropped);
 + else
 + __connman_service_notify(ipdevice-config_ipv6,
 + ipdevice-rx_packets, 
 ipdevice-tx_packets,
 + ipdevice-rx_bytes, ipdevice-tx_bytes,
 + ipdevice-rx_errors, 
 ipdevice-tx_errors,
 + ipdevice-rx_dropped, 
 ipdevice-tx_dropped);
  }
Here you could prepare a separate patch that would change the
__connman_service_notify() prototype to take a service instead of an ipconfig
as its first argument. Then you'd get the service from either your ipv4 or
ipv6 config and pass it to __connman_service_notify() here. The code would be
more readable.


 +static inline gint check_duplicate_address(gconstpointer a, gconstpointer b)
 +{
 + const struct connman_ipaddress *addr1 = a;
 + const struct connman_ipaddress *addr2 = b;
 +
 + if (addr1-prefixlen != addr2-prefixlen)
 + return addr2-prefixlen - addr1-prefixlen;
 +
 + return g_strcmp0(addr1-local, addr2-local);
 +}
 +
  void __connman_ipconfig_newaddr(int index, int family, const char *label,
   unsigned char prefixlen, const char *address)
  {
 @@ -614,26 +702,34 @@ void __connman_ipconfig_newaddr(int index, int family, 
 const char *label,
   ipaddress-prefixlen = prefixlen;
   ipaddress-local = g_strdup(address);
  
 + if (g_slist_find_custom(ipdevice-address_list, ipaddress,
 + check_duplicate_address)) {
 + connman_ipaddress_free(ipaddress);
 + return;
 + }
 +
Those 2 chunks should definitely be part of a separate patch.


   ipdevice-address_list = g_slist_append(ipdevice-address_list,
   ipaddress);
  
 - connman_info(%s {add} address %s/%u label %s, ipdevice-ifname,
 - address, prefixlen, label);
 + connman_info(%s {add} address %s/%u label %s family %d,
 + ipdevice-ifname, address, prefixlen, label, family);
  
 - if (ipdevice-config != NULL) {
 - if (family == AF_INET6  ipdevice-config-ipv6 != NULL)
 - connman_ipaddress_copy(ipdevice-config-ipv6-system,
 + if (family == AF_INET) {
 + if (ipdevice-config_ipv4 != NULL)
Please keep the same formatting: if (a  b) instead of:
if (a)

[PATCH IPv6] Separate IPv4 and IPv6 in struct connman_ipconfig so that connman can work with IPv6 only networks.

2010-11-22 Thread Jukka Rissanen
---
 include/ipconfig.h |3 +-
 include/network.h  |4 +-
 plugins/ofono.c|4 +-
 src/connman.h  |   13 ++-
 src/ipconfig.c |  381 +---
 src/ipv4.c |2 +-
 src/network.c  |  150 +
 src/provider.c |6 +-
 src/service.c  |  320 ++--
 9 files changed, 609 insertions(+), 274 deletions(-)

diff --git a/include/ipconfig.h b/include/ipconfig.h
index 28a3d6a..92110e0 100644
--- a/include/ipconfig.h
+++ b/include/ipconfig.h
@@ -74,7 +74,8 @@ struct connman_ipconfig_ops {
void (*ip_release) (struct connman_ipconfig *ipconfig);
 };
 
-struct connman_ipconfig *connman_ipconfig_create(int index);
+struct connman_ipconfig *connman_ipconfig_create(int index,
+   enum connman_ipconfig_type type);
 struct connman_ipconfig *connman_ipconfig_clone(struct connman_ipconfig 
*ipconfig);
 struct connman_ipconfig *connman_ipconfig_ref(struct connman_ipconfig 
*ipconfig);
 void connman_ipconfig_unref(struct connman_ipconfig *ipconfig);
diff --git a/include/network.h b/include/network.h
index f6ebc3a..0b538a4 100644
--- a/include/network.h
+++ b/include/network.h
@@ -89,7 +89,9 @@ connman_bool_t connman_network_get_connected(struct 
connman_network *network);
 
 connman_bool_t connman_network_get_associating(struct connman_network 
*network);
 
-void connman_network_set_method(struct connman_network *network,
+void connman_network_set_ipv4_method(struct connman_network *network,
+   enum connman_ipconfig_method method);
+void connman_network_set_ipv6_method(struct connman_network *network,
enum connman_ipconfig_method method);
 
 int connman_network_set_address(struct connman_network *network,
diff --git a/plugins/ofono.c b/plugins/ofono.c
index 691037d..e9c5e71 100644
--- a/plugins/ofono.c
+++ b/plugins/ofono.c
@@ -1484,7 +1484,7 @@ static void set_connected(struct connman_network *network,
return;
 
case CONNMAN_IPCONFIG_METHOD_FIXED:
-   connman_network_set_method(network, method);
+   connman_network_set_ipv4_method(network, method);
 
if (connected == FALSE)
cleanup_ipconfig(network);
@@ -1493,7 +1493,7 @@ static void set_connected(struct connman_network *network,
break;
 
case CONNMAN_IPCONFIG_METHOD_DHCP:
-   connman_network_set_method(network, method);
+   connman_network_set_ipv4_method(network, method);
 
connman_network_set_connected(network, connected);
break;
diff --git a/src/connman.h b/src/connman.h
index 8cb4754..1a781ce 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -263,6 +263,8 @@ int __connman_ipconfig_load(struct connman_ipconfig 
*ipconfig,
GKeyFile *keyfile, const char *identifier, const char *prefix);
 int __connman_ipconfig_save(struct connman_ipconfig *ipconfig,
GKeyFile *keyfile, const char *identifier, const char *prefix);
+int __connman_ipconfig_get_configs(int index, struct connman_ipconfig **ipv4,
+   struct connman_ipconfig **ipv6);
 
 #include connman/utsname.h
 
@@ -374,7 +376,8 @@ int __connman_network_disconnect(struct connman_network 
*network);
 int __connman_network_clear_ipconfig(struct connman_network *network,
struct connman_ipconfig *ipconfig);
 int __connman_network_set_ipconfig(struct connman_network *network,
-   struct connman_ipconfig *ipconfig);
+   struct connman_ipconfig *ipconfig_ipv4,
+   struct connman_ipconfig *ipconfig_ipv6);
 
 connman_bool_t __connman_network_has_driver(struct connman_network *network);
 
@@ -455,9 +458,13 @@ struct connman_service 
*__connman_service_create_from_provider(struct connman_pr
 void __connman_service_update_from_network(struct connman_network *network);
 void __connman_service_remove_from_network(struct connman_network *network);
 
-void __connman_service_create_ipconfig(struct connman_service *service,
+void __connman_service_create_ip4config(struct connman_service *service,
+   int index);
+void __connman_service_create_ip6config(struct connman_service *service,
int index);
-struct connman_ipconfig *__connman_service_get_ipconfig(
+struct connman_ipconfig *__connman_service_get_ip4config(
+   struct connman_service *service);
+struct connman_ipconfig *__connman_service_get_ip6config(
struct connman_service *service);
 const char *__connman_service_get_ident(struct connman_service *service);
 const char