Re: [help] Implementation of 2 internet contexts

2015-10-01 Thread Mylene Josserand
Hi Patrik,

Thank you for your quick answer and your help.


> On Wed, 2015-09-30 at 17:13 +0200, Mylene Josserand wrote:
>> > On 05/13/2015 01:10 PM, Tomasz Bursztyka wrote:
>> >> Hi Mylene,
>> >>
>> >>> I understand and see (in outline)  how adapt the code.
>> >>> Thank you for the tips, it will help me a lot if the Frederik solution's 
>> >>> did not
>> >>> work for me.
>> >>
>> >> Looks like Frederik's solution works with either context A or context B, 
>> >> not
>> >> both at same time.
>> >>
>> >> Anyway, if you get a patch that works, you will be welcomed to send it on 
>> >> this
>> >> ML.
>> > 
>> > Okay, I will let you know if I managed to get it work.
>> > 
>> 
>> 
>> Sorry for my (very) late answer.
> 
> We're fine with a bit longer real world schedules, no worries here.
> 
>> I have managed to handle two (or more) internet contexts at the same
>> time, 2-3 days after my last post but I was busy so I did not have
>> time to send a patch.
>> 
>> I have implemented it (and tested it) with version 1.29. I am
>> currently merging it to the last version of connman.
>> 
>> There are many modifications and I do not really know how I should
>> categorize them into patches series. As I removed the "network"
>> property in modem_data and added it into context_data, there are many
>> functions that are impacted.
> 
> Ideally it should be be possible to present this kind of major
> structural change with moving data from one structure to another by
> creating a bigger patch that does only that with no loss of
> functionality. Out of necessity this patch then touches quite many
> places at the same time. Ideally the next (or previous) patch(es) in the
> series would then add the desired feature change(s). But as you propose
> below,...


Okay, I understand.



>>  If it is okay for you, I thought I could send one patch with all
>> modifications and if there is a way to categorize, you could report it
>> to me.
> 
> ...you can also send one big patch where we can help you categorizing
> and splitting up the functionality into smaller patches. To help us
> remember this still next week, add 'RFC' to the patch subject and write
> the commit or cover letter message to indicate that you are asking for
> input in splitting up this patch.


I think I found 3 "categories" so I will send a patch series (but I will add 
the RFC flag as I am not sure about it).


> 
>> I tried to follow the coding style and I checked the code with
>> checkpatch.pl script from Linux kernel. I hope it will be okay.
> 
> Kernel coding style is mostly ok, we'll do any additional nitpicking
> after or during split up. I'd concentrate more on the patch splitting
> first with nitpicking later. Causes one more round, but is probably less
> confusing this way.


Okay, it is fine for me.


Thank you !

Mylène
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] IPv6 Timeserver for NTP

2015-10-01 Thread Naveen Singh
On Thu, Oct 1, 2015 at 6:26 AM, Patrik Flykt 
wrote:

>
> Hi,
>
> On Wed, 2015-09-30 at 22:08 -0700, Naveen Singh wrote:
> > From: Naveen Singh 
> >
> > Current NTP code is written with an assumption that timeserver is
> > always an IPv4 address. If there is an IPv6 timeserver then the socket
> > operation would fail with error as Permission denied (13). This change in
> > ntp.c ensures that code works fine with both IPv4 and IPv6 address.
> > ---
> >  src/ntp.c | 129
> --
> >  1 file changed, 101 insertions(+), 28 deletions(-)
>
> This was patch version 2, so the next one is version 3. Add the version
> to the Subject: line as [PATCH v3], this can be done with
> 'git ... --subject-prefix="PATCH v3" ...'.
>
> > diff --git a/src/ntp.c b/src/ntp.c
> > index 2c313a4..a55365d 100644
> > --- a/src/ntp.c
> > +++ b/src/ntp.c
> > @@ -18,7 +18,6 @@
> >   *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> 02110-1301  USA
> >   *
> >   */
> > -
> >  #ifdef HAVE_CONFIG_H
> >  #include 
> >  #endif
> > @@ -34,6 +33,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #include 
> >
> > @@ -117,12 +117,12 @@ static struct timespec mtx_time;
> >  static int transmit_fd = 0;
> >
> >  static char *timeserver = NULL;
> > -static struct sockaddr_in timeserver_addr;
> > +static struct sockaddr_storage timeserver_addr;
> >  static gint poll_id = 0;
> >  static gint timeout_id = 0;
> >  static guint retries = 0;
> >
> > -static void send_packet(int fd, const char *server, uint32_t timeout);
> > +static void send_packet(int fd, const char *server, uint32_t timeout,
> int family);
> >
> >  static void next_server(void)
> >  {
> > @@ -143,17 +143,21 @@ static gboolean send_timeout(gpointer user_data)
> >   if (retries++ == NTP_SEND_RETRIES)
> >   next_server();
> >   else
> > - send_packet(transmit_fd, timeserver, timeout << 1);
> > + send_packet(transmit_fd, timeserver, timeout << 1,
> timeserver_addr.ss_family);
> >
> >   return FALSE;
> >  }
> >
> > -static void send_packet(int fd, const char *server, uint32_t timeout)
> > +static void send_packet(int fd, const char *server, uint32_t timeout,
> int family)
>
> Instead of supplying a char *server, why don't we give a struct
> sockaddr* instead? What's the purpose of carrying the server
> "name" (nah, the IP address as a string), if it is already resolved in
> the calling function? BTW, it is available as timeserver_addr, so do we
> really need any of this information here?
>

Yes it makes sense but I am not sure sockaddr * is the one we should us. It
will not be able to hold IPv6 address which is 16 bytes. So I guess
sockaddr_storage* would be the right thing. Do you agree? If you look i
changed type of timeserver_addr to sockaddr_storage. Do you agree with this?

/* Structure describing a generic socket address.  */
struct sockaddr
  {
__SOCKADDR_COMMON (sa_);/* Common data: address family and length.
 */
char sa_data[14];   /* Address data.  */
  };



> >  {
> >   struct ntp_msg msg;
> > - struct sockaddr_in addr;
> > + struct sockaddr_in in4addr;
> > + struct sockaddr_in6 in6addr;
> > + struct sockaddr * addr;
> >   struct timeval transmit_timeval;
> >   ssize_t len;
> > + int size;
> > + unsigned char * addrptr;
> >
> >   /*
> >* At some point, we could specify the actual system precision
> with:
> > @@ -168,10 +172,29 @@ static void send_packet(int fd, const char
> *server, uint32_t timeout)
> >   msg.poll = 10;  // max
> >   msg.precision = NTP_PRECISION_S;
> >
> > - memset(&addr, 0, sizeof(addr));
> > - addr.sin_family = AF_INET;
> > - addr.sin_port = htons(123);
> > - addr.sin_addr.s_addr = inet_addr(server);
> > + if (family == AF_INET) {
> > + memset(&in4addr, 0, sizeof(in4addr));
> > + in4addr.sin_family = AF_INET;
> > + in4addr.sin_port = htons(123);
> > + size = sizeof(in4addr);
> > + addrptr = (unsigned char *)&in4addr.sin_addr.s_addr;
> > + addr = (struct sockaddr *)&in4addr;
> > + } else if (family == AF_INET6){
> > + memset(&in6addr, 0, sizeof(in6addr));
> > + in6addr.sin6_family = AF_INET6;
> > + in6addr.sin6_port = htons(123);
> > + size = sizeof(in6addr);
> > + addrptr = in6addr.sin6_addr.__in6_u.__u6_addr8;
> > + addr = (struct sockaddr *)&in6addr;
> > + } else {
> > + DBG("wrong family type");
> > + return;
> > + }
> > + if (inet_pton(family, server, addrptr) == 0)
> > + {
> > + DBG("cannot convert ip address string");
> > + return;
> > + }
>
> The above is unnecessary. You already have timeserver_addr set or the
> function gets a sockaddr * to use.
>

Agreed. If we pass sockaddr_storage*.

>
> >   ge

Glib main loop stops dispatching

2015-10-01 Thread Naveen Singh
Hi
We have been seeing a hang in connman regularly while running a wifi
disconnect-connect test. The last log line we see is a connman error log:
"*Invalid packet timestamp from time server*"

After that we do not see any log messages from connman. I debugged this by
doing following:
1. Dump the payload bytes when this condition happens.
2. Ran a wireless capture to find out which packet it is?

I found that payload was for a  DNS response for ipv6.connman.net. It
does look like somehow glib main loop is not dispatching to correct
handlers and then it stops dispatching causing connman to hang.

Has anyone seen any problem like this. I could provide complete logs for
this. Just before this happens there is a pending wispr_portal_request.

Regards
Naveen
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: [PATCH] IPv6 Timeserver for NTP

2015-10-01 Thread Naveen Singh
On Thu, Oct 1, 2015 at 6:26 AM, Patrik Flykt 
wrote:

>
> Hi,
>
> On Wed, 2015-09-30 at 22:08 -0700, Naveen Singh wrote:
> > From: Naveen Singh 
> >
> > Current NTP code is written with an assumption that timeserver is
> > always an IPv4 address. If there is an IPv6 timeserver then the socket
> > operation would fail with error as Permission denied (13). This change in
> > ntp.c ensures that code works fine with both IPv4 and IPv6 address.
> > ---
> >  src/ntp.c | 129
> --
> >  1 file changed, 101 insertions(+), 28 deletions(-)
>
> This was patch version 2, so the next one is version 3. Add the version
> to the Subject: line as [PATCH v3], this can be done with
> 'git ... --subject-prefix="PATCH v3" ...'.
>
> > diff --git a/src/ntp.c b/src/ntp.c
> > index 2c313a4..a55365d 100644
> > --- a/src/ntp.c
> > +++ b/src/ntp.c
> > @@ -18,7 +18,6 @@
> >   *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> 02110-1301  USA
> >   *
> >   */
> > -
> >  #ifdef HAVE_CONFIG_H
> >  #include 
> >  #endif
> > @@ -34,6 +33,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #include 
> >
> > @@ -117,12 +117,12 @@ static struct timespec mtx_time;
> >  static int transmit_fd = 0;
> >
> >  static char *timeserver = NULL;
> > -static struct sockaddr_in timeserver_addr;
> > +static struct sockaddr_storage timeserver_addr;
> >  static gint poll_id = 0;
> >  static gint timeout_id = 0;
> >  static guint retries = 0;
> >
> > -static void send_packet(int fd, const char *server, uint32_t timeout);
> > +static void send_packet(int fd, const char *server, uint32_t timeout,
> int family);
> >
> >  static void next_server(void)
> >  {
> > @@ -143,17 +143,21 @@ static gboolean send_timeout(gpointer user_data)
> >   if (retries++ == NTP_SEND_RETRIES)
> >   next_server();
> >   else
> > - send_packet(transmit_fd, timeserver, timeout << 1);
> > + send_packet(transmit_fd, timeserver, timeout << 1,
> timeserver_addr.ss_family);
> >
> >   return FALSE;
> >  }
> >
> > -static void send_packet(int fd, const char *server, uint32_t timeout)
> > +static void send_packet(int fd, const char *server, uint32_t timeout,
> int family)
>
> Instead of supplying a char *server, why don't we give a struct
> sockaddr* instead? What's the purpose of carrying the server
> "name" (nah, the IP address as a string), if it is already resolved in
> the calling function? BTW, it is available as timeserver_addr, so do we
> really need any of this information here?
>
> >  {
> >   struct ntp_msg msg;
> > - struct sockaddr_in addr;
> > + struct sockaddr_in in4addr;
> > + struct sockaddr_in6 in6addr;
> > + struct sockaddr * addr;
> >   struct timeval transmit_timeval;
> >   ssize_t len;
> > + int size;
> > + unsigned char * addrptr;
> >
> >   /*
> >* At some point, we could specify the actual system precision
> with:
> > @@ -168,10 +172,29 @@ static void send_packet(int fd, const char
> *server, uint32_t timeout)
> >   msg.poll = 10;  // max
> >   msg.precision = NTP_PRECISION_S;
> >
> > - memset(&addr, 0, sizeof(addr));
> > - addr.sin_family = AF_INET;
> > - addr.sin_port = htons(123);
> > - addr.sin_addr.s_addr = inet_addr(server);
> > + if (family == AF_INET) {
> > + memset(&in4addr, 0, sizeof(in4addr));
> > + in4addr.sin_family = AF_INET;
> > + in4addr.sin_port = htons(123);
> > + size = sizeof(in4addr);
> > + addrptr = (unsigned char *)&in4addr.sin_addr.s_addr;
> > + addr = (struct sockaddr *)&in4addr;
> > + } else if (family == AF_INET6){
> > + memset(&in6addr, 0, sizeof(in6addr));
> > + in6addr.sin6_family = AF_INET6;
> > + in6addr.sin6_port = htons(123);
> > + size = sizeof(in6addr);
> > + addrptr = in6addr.sin6_addr.__in6_u.__u6_addr8;
> > + addr = (struct sockaddr *)&in6addr;
> > + } else {
> > + DBG("wrong family type");
> > + return;
> > + }
> > + if (inet_pton(family, server, addrptr) == 0)
> > + {
> > + DBG("cannot convert ip address string");
> > + return;
> > + }
>
> The above is unnecessary. You already have timeserver_addr set or the
> function gets a sockaddr * to use.
>
> >   gettimeofday(&transmit_timeval, NULL);
> >   clock_gettime(CLOCK_MONOTONIC, &mtx_time);
> > @@ -180,7 +203,7 @@ static void send_packet(int fd, const char *server,
> uint32_t timeout)
> >   msg.xmttime.fraction = htonl(transmit_timeval.tv_usec * 1000);
> >
> >   len = sendto(fd, &msg, sizeof(msg), MSG_DONTWAIT,
> > - &addr, sizeof(addr));
> > + addr, size);
>
> Use the stored timeserver_addr or supplied sockaddr * family information
> to set the siz

Re: [PATCH] IPv6 Timeserver for NTP

2015-10-01 Thread Patrik Flykt

Hi,

On Wed, 2015-09-30 at 22:08 -0700, Naveen Singh wrote:
> From: Naveen Singh 
> 
> Current NTP code is written with an assumption that timeserver is
> always an IPv4 address. If there is an IPv6 timeserver then the socket
> operation would fail with error as Permission denied (13). This change in
> ntp.c ensures that code works fine with both IPv4 and IPv6 address.
> ---
>  src/ntp.c | 129 
> --
>  1 file changed, 101 insertions(+), 28 deletions(-)

This was patch version 2, so the next one is version 3. Add the version
to the Subject: line as [PATCH v3], this can be done with
'git ... --subject-prefix="PATCH v3" ...'.

> diff --git a/src/ntp.c b/src/ntp.c
> index 2c313a4..a55365d 100644
> --- a/src/ntp.c
> +++ b/src/ntp.c
> @@ -18,7 +18,6 @@
>   *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  
> USA
>   *
>   */
> -
>  #ifdef HAVE_CONFIG_H
>  #include 
>  #endif
> @@ -34,6 +33,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  
> @@ -117,12 +117,12 @@ static struct timespec mtx_time;
>  static int transmit_fd = 0;
>  
>  static char *timeserver = NULL;
> -static struct sockaddr_in timeserver_addr;
> +static struct sockaddr_storage timeserver_addr;
>  static gint poll_id = 0;
>  static gint timeout_id = 0;
>  static guint retries = 0;
>  
> -static void send_packet(int fd, const char *server, uint32_t timeout);
> +static void send_packet(int fd, const char *server, uint32_t timeout, int 
> family);
>  
>  static void next_server(void)
>  {
> @@ -143,17 +143,21 @@ static gboolean send_timeout(gpointer user_data)
>   if (retries++ == NTP_SEND_RETRIES)
>   next_server();
>   else
> - send_packet(transmit_fd, timeserver, timeout << 1);
> + send_packet(transmit_fd, timeserver, timeout << 1, 
> timeserver_addr.ss_family);
>  
>   return FALSE;
>  }
>  
> -static void send_packet(int fd, const char *server, uint32_t timeout)
> +static void send_packet(int fd, const char *server, uint32_t timeout, int 
> family)

Instead of supplying a char *server, why don't we give a struct
sockaddr* instead? What's the purpose of carrying the server
"name" (nah, the IP address as a string), if it is already resolved in
the calling function? BTW, it is available as timeserver_addr, so do we
really need any of this information here?

>  {
>   struct ntp_msg msg;
> - struct sockaddr_in addr;
> + struct sockaddr_in in4addr;
> + struct sockaddr_in6 in6addr;
> + struct sockaddr * addr;
>   struct timeval transmit_timeval;
>   ssize_t len;
> + int size;
> + unsigned char * addrptr;
>  
>   /*
>* At some point, we could specify the actual system precision with:
> @@ -168,10 +172,29 @@ static void send_packet(int fd, const char *server, 
> uint32_t timeout)
>   msg.poll = 10;  // max
>   msg.precision = NTP_PRECISION_S;
>  
> - memset(&addr, 0, sizeof(addr));
> - addr.sin_family = AF_INET;
> - addr.sin_port = htons(123);
> - addr.sin_addr.s_addr = inet_addr(server);
> + if (family == AF_INET) {
> + memset(&in4addr, 0, sizeof(in4addr));
> + in4addr.sin_family = AF_INET;
> + in4addr.sin_port = htons(123);
> + size = sizeof(in4addr);
> + addrptr = (unsigned char *)&in4addr.sin_addr.s_addr;
> + addr = (struct sockaddr *)&in4addr;
> + } else if (family == AF_INET6){
> + memset(&in6addr, 0, sizeof(in6addr));
> + in6addr.sin6_family = AF_INET6;
> + in6addr.sin6_port = htons(123);
> + size = sizeof(in6addr);
> + addrptr = in6addr.sin6_addr.__in6_u.__u6_addr8;
> + addr = (struct sockaddr *)&in6addr;
> + } else {
> + DBG("wrong family type");
> + return;
> + }
> + if (inet_pton(family, server, addrptr) == 0)
> + {
> + DBG("cannot convert ip address string");
> + return;
> + }

The above is unnecessary. You already have timeserver_addr set or the
function gets a sockaddr * to use.

>   gettimeofday(&transmit_timeval, NULL);
>   clock_gettime(CLOCK_MONOTONIC, &mtx_time);
> @@ -180,7 +203,7 @@ static void send_packet(int fd, const char *server, 
> uint32_t timeout)
>   msg.xmttime.fraction = htonl(transmit_timeval.tv_usec * 1000);
>  
>   len = sendto(fd, &msg, sizeof(msg), MSG_DONTWAIT,
> - &addr, sizeof(addr));
> + addr, size);

Use the stored timeserver_addr or supplied sockaddr * family information
to set the size to either sizeof(struct sockaddr_in) or sizeof(struct
sockaddr_in6).

>   if (len < 0) {
>   connman_error("Time request for server %s failed (%d/%s)",
>   server, errno, strerror(errno));
> @@ -213,7 +236,7 @@ static gboolean next_poll(gpoin

Re: [PATCH] add management of dsa interface

2015-10-01 Thread Patrik Flykt

Hi,

git am complains of trailing white space errors on lines 47 and 56.
While looking at the patch, all lines end with ^M, please fix.

On Mon, 2015-09-28 at 14:48 +0200, Laurent Vaudoit wrote:
> ---
>  plugins/ethernet.c | 60 
> +++---
>  1 file changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/plugins/ethernet.c b/plugins/ethernet.c
> index d176508..456d3cc 100644
> --- a/plugins/ethernet.c
> +++ b/plugins/ethernet.c
> @@ -32,6 +32,7 @@
>  
>  #include 
>  #include 
> +#include 
>  
>  #ifndef IFF_LOWER_UP
>  #define IFF_LOWER_UP 0x1
> @@ -83,6 +84,54 @@ static int get_vlan_vid(const char *ifname)
>   return vid;
>  }
>  
> +static int get_dsa_port(const char *ifname)
> +{
> + int sk;
> + int dsaport=-1;

Spaces before and after =

> + struct ifreq ifr;
> + struct ethtool_cmd cmd;
> + struct ethtool_drvinfo drvinfocmd;
> + struct vlan_ioctl_args vifr;
> +
> + sk = socket(AF_INET, SOCK_STREAM, 0);
> + if (sk < 0)
> + return -errno;
> +
> + memset(&ifr, 0, sizeof(ifr));
> + strcpy(ifr.ifr_name, ifname);

ifr.ifr_name probably has a max lenght and I don't know if it must end
in a \0. If it does not defined to end with a \0 or max length is
specified, a strncpy should be used instead.

> +
> + /* check if it is a vlan and get physical interface name*/
> + vifr.cmd = GET_VLAN_REALDEV_NAME_CMD;
> + strncpy(vifr.device1, ifname, sizeof(vifr.device1));
> +
> + if(ioctl(sk, SIOCSIFVLAN, &vifr) >= 0)
> + strcpy(ifr.ifr_name, vifr.u.device2);

If this failed, can we return here? Is strcpy safe here for all string
lengths as above?

> +
> + /* get driver info */
> + drvinfocmd.cmd =  ETHTOOL_GDRVINFO;
> + ifr.ifr_data = (caddr_t)&drvinfocmd;
> + 
> + /*  ioctl failed:   */
> + if (ioctl(sk, SIOCETHTOOL, &ifr) != 0)
> + DBG("Cannot get driver infos\n");

Could this be turned around without the debug, as the more likely case
is that this always works, and with the error returned in the return
value. One can later on see that it worked, as the identifier is
different. And the comment is also a bit unnecessary then.

Compare zero with '!'.

> + else {
> + if(strcmp(drvinfocmd.driver,"dsa") == 0) {
> + /* get dsa port*/
> + cmd.cmd =  ETHTOOL_GSET;
> + ifr.ifr_data = (caddr_t)&cmd;
> + 
> + /*  ioctl failed:   */
> + if (ioctl(sk, SIOCETHTOOL, &ifr) != 0)
> + DBG("Cannot get device settings\n");

Same here.

> + else
> + dsaport = cmd.phy_address;
> + }
> + }
> + close(sk);
> +
> + return dsaport;
> +}
> +
>  static int eth_network_probe(struct connman_network *network)
>  {
>   DBG("network %p", network);
> @@ -126,7 +175,7 @@ static void add_network(struct connman_device *device,
>   struct ethernet_data *ethernet)
>  {
>   struct connman_network *network;
> - int index, vid;
> + int index, vid,dsaport;

Nitpick: space after comma.

>   char *ifname;
>  
>   network = connman_network_create("carrier",
> @@ -140,6 +189,7 @@ static void add_network(struct connman_device *device,
>   if (!ifname)
>   return;
>   vid = get_vlan_vid(ifname);
> + dsaport = get_dsa_port(ifname);
>  
>   connman_network_set_name(network, "Wired");
>  
> @@ -149,14 +199,18 @@ static void add_network(struct connman_device *device,
>   }
>  
>   if (!eth_tethering) {
> - char group[10] = "cable";
> + char group[16] = "cable";
>   /*
>* Prevent service from starting the reconnect
>* procedure as we do not want the DHCP client
>* to run when tethering.
>*/
> - if (vid >= 0)
> + if((vid >= 0) && (dsaport >= 0))

dsaport need only be assigned here, can you move it down here please. I
just noticed that then vid is also misplaced, but don't worry about
that.

> + snprintf(group, sizeof(group), "p%02x_%03x_cable", 
> dsaport, vid);
> + else if (vid >= 0)
>   snprintf(group, sizeof(group), "%03x_cable", vid);
> + else if (dsaport >= 0)
> + snprintf(group, sizeof(group), "p%02x_cable", dsaport);
>  
>   connman_network_set_group(network, group);
>   }


Patrik


___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: [help] Implementation of 2 internet contexts

2015-10-01 Thread Patrik Flykt

Hi,

On Wed, 2015-09-30 at 17:13 +0200, Mylene Josserand wrote:
> Hi everyone,
> 
> 
> > On 05/13/2015 01:10 PM, Tomasz Bursztyka wrote:
> >> Hi Mylene,
> >>
> >>> I understand and see (in outline)  how adapt the code.
> >>> Thank you for the tips, it will help me a lot if the Frederik solution's 
> >>> did not
> >>> work for me.
> >>
> >> Looks like Frederik's solution works with either context A or context B, 
> >> not
> >> both at same time.
> >>
> >> Anyway, if you get a patch that works, you will be welcomed to send it on 
> >> this
> >> ML.
> > 
> > Okay, I will let you know if I managed to get it work.
> > 
> 
> 
> Sorry for my (very) late answer.

We're fine with a bit longer real world schedules, no worries here.

> I have managed to handle two (or more) internet contexts at the same
> time, 2-3 days after my last post but I was busy so I did not have
> time to send a patch.
> 
> I have implemented it (and tested it) with version 1.29. I am
> currently merging it to the last version of connman.
> 
> There are many modifications and I do not really know how I should
> categorize them into patches series. As I removed the "network"
> property in modem_data and added it into context_data, there are many
> functions that are impacted.

Ideally it should be be possible to present this kind of major
structural change with moving data from one structure to another by
creating a bigger patch that does only that with no loss of
functionality. Out of necessity this patch then touches quite many
places at the same time. Ideally the next (or previous) patch(es) in the
series would then add the desired feature change(s). But as you propose
below,...

>  If it is okay for you, I thought I could send one patch with all
> modifications and if there is a way to categorize, you could report it
> to me.

...you can also send one big patch where we can help you categorizing
and splitting up the functionality into smaller patches. To help us
remember this still next week, add 'RFC' to the patch subject and write
the commit or cover letter message to indicate that you are asking for
input in splitting up this patch.

> I tried to follow the coding style and I checked the code with
> checkpatch.pl script from Linux kernel. I hope it will be okay.

Kernel coding style is mostly ok, we'll do any additional nitpicking
after or during split up. I'd concentrate more on the patch splitting
first with nitpicking later. Causes one more round, but is probably less
confusing this way.

> I will send the patch in next few days. It will be the first version
> so do not hesitate for any comments. I would send a version 2 with
> pleasure.

Thanks!

Patrik

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman