Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. Re: [PATCH 1/2] ntp: Fix potentially leaked addrinfo
(Patrik Flykt)
2. Re: [PATCH 2/2] dhcp: Remove dead code (Patrik Flykt)
3. Re: Not sure how to fix this bug (Patrik Flykt)
4. Re: [PATCH 2/3] Check for in6_pktinfo.ipi6_addr explicitly
(Patrik Flykt)
5. Re: [PATCH 3/3] Rationalise includes (Patrik Flykt)
6. Re: Add speed information to services (Marcel Holtmann)
7. Re: [PATCH 1/3] technology: Add specific D-Bus methods for
WPS connections (Jose Blanquicet)
----------------------------------------------------------------------
Message: 1
Date: Mon, 29 Aug 2016 13:06:01 +0300
From: Patrik Flykt <[email protected]>
To: Peter Meerwald-Stadler <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 1/2] ntp: Fix potentially leaked addrinfo
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
On Thu, 2016-08-18 at 18:18 +0200, Peter Meerwald-Stadler wrote:
> CID 1352476
> ---
> ?src/ntp.c | 5 ++---
> ?1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/src/ntp.c b/src/ntp.c
> index 9b1bb4f..bb15792 100644
> --- a/src/ntp.c
> +++ b/src/ntp.c
> @@ -477,15 +477,15 @@ static void start_ntp(char *server)
> ? hint.ai_socktype = SOCK_DGRAM;
> ? hint.ai_flags = AI_NUMERICHOST | AI_PASSIVE;
> ? ret = getaddrinfo(server, NULL, &hint, &info);
> -
> ? if (ret) {
> ? connman_error("cannot get server info");
> ? return;
> ? }
> ?
> ? family = info->ai_family;
> -
Hmm, empty lines need not be removed when fixing a potential memory
leak. Could you also tune the commit message a bit, I started to think
that it was about privacy when I first read it...
Cheers,
Patrik
> ? memcpy(×erver_addr, info->ai_addr, info->ai_addrlen);
> + freeaddrinfo(info);
> +
> ? memset(&in6addr, 0, sizeof(in6addr));
> ?
> ? if (family == AF_INET) {
> @@ -503,7 +503,6 @@ static void start_ntp(char *server)
> ? connman_error("Family is neither ipv4 nor ipv6");
> ? return;
> ? }
> - freeaddrinfo(info);
> ?
> ? DBG("server %s family %d", server, family);
> ?
------------------------------
Message: 2
Date: Mon, 29 Aug 2016 13:07:20 +0300
From: Patrik Flykt <[email protected]>
To: Peter Meerwald-Stadler <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 2/2] dhcp: Remove dead code
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
On Thu, 2016-08-18 at 18:18 +0200, Peter Meerwald-Stadler wrote:
> CID 1230769
> ---
> ?gdhcp/client.c | 4 ----
> ?1 file changed, 4 deletions(-)
>
> diff --git a/gdhcp/client.c b/gdhcp/client.c
> index fbb40ab..e9e38e7 100644
> --- a/gdhcp/client.c
> +++ b/gdhcp/client.c
> @@ -2351,10 +2351,6 @@ static gboolean listener_event(GIOChannel
> *channel, GIOCondition condition,
> ? return TRUE;
> ? }
> ?
> - if (!message_type && !client_id)
> - /* No message type / client id option, ignore
> package */
> - return TRUE;
> -
Please describe in the commit message how/why this is dead code.
Cheers,
Patrik
> ? debug(dhcp_client, "received DHCP packet xid 0x%04x "
> ? "(current state %d)", ntohl(xid), dhcp_client-
> >state);
> ?
------------------------------
Message: 3
Date: Mon, 29 Aug 2016 13:20:04 +0300
From: Patrik Flykt <[email protected]>
To: Thomas Green <[email protected]>, "[email protected]"
<[email protected]>
Subject: Re: Not sure how to fix this bug
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
On Fri, 2016-08-26 at 22:28 +0000, Thomas Green wrote:
> Sorry to follow up my own post, but as no one answered, I am
> attaching my own proposed fix.? If this looks ok, let me know.
Thanks for the patch, now I have a decent idea what the bug might be
about.
Patrik
------------------------------
Message: 4
Date: Mon, 29 Aug 2016 13:23:38 +0300
From: Patrik Flykt <[email protected]>
To: Ross Burton <[email protected]>, [email protected]
Subject: Re: [PATCH 2/3] Check for in6_pktinfo.ipi6_addr explicitly
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
On Tue, 2016-08-09 at 17:00 +0100, Ross Burton wrote:
> Instead of assuming that just glibc has this structure, check for it
> at configure as musl also has it.
>
> Based on work by Khem Raj <[email protected]>.
I got this from gcc:
In file included from gdhcp/client.c:47:0:
gdhcp/common.h:176:8: error: redefinition of ?struct in6_pktinfo?
?struct in6_pktinfo {
????????^
In file included from /usr/include/arpa/inet.h:22:0,
?????????????????from gdhcp/client.c:33:
/usr/include/netinet/in.h:535:8: note: originally defined here
?struct in6_pktinfo
????????^
? CC???????gsupplicant/src_connmand-supplicant.o
? CC???????gsupplicant/src_connmand-dbus.o
Makefile:2916: recipe for target 'gdhcp/src_connmand-client.o' failed
make[1]: *** [gdhcp/src_connmand-client.o] Error 1
make[1]: *** Waiting for unfinished jobs....
? CC???????plugins/src_connmand-bluetooth.o
Makefile:1623: recipe for target 'all' failed
make: *** [all] Error 2
Cheers,
Patrik
> ---
> ?configure.ac???| 2 ++
> ?gdhcp/common.h | 5 +++--
> ?2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index bacf5ec..ad00456 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -186,6 +186,8 @@ AC_CHECK_LIB(resolv, ns_initparse, dummy=yes, [
> ?AC_CHECK_HEADERS([execinfo.h])
> ?AM_CONDITIONAL([BACKTRACE], [test "${ac_cv_header_execinfo_h}" =
> "yes"])
> ?
> +AC_CHECK_MEMBERS([struct in6_pktinfo.ipi6_addr], [], [], [[#include
> <netinet/in.h>]])
> +
> ?AC_CHECK_FUNC(signalfd, dummy=yes,
> ? AC_MSG_ERROR(signalfd support is required))
> ?
> diff --git a/gdhcp/common.h b/gdhcp/common.h
> index 75abc18..6899499 100644
> --- a/gdhcp/common.h
> +++ b/gdhcp/common.h
> @@ -19,6 +19,7 @@
> ? *
> ? */
> ?
> +#include <config.h>
> ?#include <netinet/udp.h>
> ?#include <netinet/ip.h>
> ?
> @@ -170,8 +171,8 @@ static const uint8_t dhcp_option_lengths[] = {
> ? [OPTION_U32] = 4,
> ?};
> ?
> -/* already defined within netinet/in.h if using GNU compiler */
> -#ifndef __USE_GNU
> +/* already defined within netinet/in.h if using glibc or musl */
> +#ifndef HAVE_STRUCT_IN6_PKTINFO_IPI6_ADDR
> ?struct in6_pktinfo {
> ? struct in6_addr ipi6_addr;??/* src/dst IPv6 address */
> ? unsigned int ipi6_ifindex;??/* send/recv interface index */
------------------------------
Message: 5
Date: Mon, 29 Aug 2016 15:46:32 +0300
From: Patrik Flykt <[email protected]>
To: Ross Burton <[email protected]>, [email protected]
Subject: Re: [PATCH 3/3] Rationalise includes
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
Hi,
On Tue, 2016-08-09 at 17:00 +0100, Ross Burton wrote:
> gweb/gresolv.c uses snprintf() and isspace() so it should include
> stdio.h and
> ctype.h.
>
> tools/dnsproxy-test uses functions from stdio.h.
This should be one patch (missing headers). Actually two, as it spans
different directories with different usage.
> musl warns when sys/ headers are included when the non-sys form
> should be used,
> so switch sys/errno.h and so on to errno.h.
>
> musl also causes redefinition errors when pieces of the networking
> headers are
> included, so remove the redundant includes.
This should be a separate one, as it is musl related. It's better to
know which were musl related changes and which were adding missing
headers.
Cheers,
Patrik
------------------------------
Message: 6
Date: Mon, 29 Aug 2016 07:17:46 -0700
From: Marcel Holtmann <[email protected]>
To: "Blanquicet-Melendez Jose (MM)"
<[email protected]>
Cc: "[email protected]" <[email protected]>, "MANIEZZO Marco
(MM)" <[email protected]>
Subject: Re: Add speed information to services
Message-ID: <[email protected]>
Content-Type: text/plain; charset=us-ascii
Hi Jose,
>>> Nowadays most of the smartphones and laptops (Windows and Linux) provide
>>> the link speed information to the users, either to allow them to choose the
>>> most suitable network to connect to as well as (After having got connected)
>>> to show the current link speed. Unfortunately, this is something not
>>> provided by ConnMan yet.
>>
>> the link speed or to be more precise the actual data rate is something that
>> is constantly changing and not really of massive use.
>
> Please look at my last comment.
>
>>> Looking at the information we have inside ConnMan to implement such a
>>> feature, we found that we already have everything to do this. First of all,
>>> wpa_supplicant provides the list of BSS supported rates from which we could
>>> know if a BSS supports "b", "g" or "a", to distinguish between "g" or "a"
>>> we can just check the band for dual-band APs cases.
>>
>> You do realize that dual-band (2.4 GHz and 5 GHz) APs are not a single BSS.
>> They are two BSS. They are actually two radios. One in 2.4 GHz and the other
>> in 5 GHz and a switch from one to the other is actually roaming. So the use
>> case of detecting dual-band APs is mood. The two radios might be in the same
>> casing, but it would make no different if they are two cases standing next
>> to each other. And ConnMan would not be able to tell the difference.
>
> You are right, they are actually two BSS but both using the same SSID,
> therefore both will belong to the same network; this is how wpa_supplicant
> handles this situation and consequently also ConnMan. As a consequence, a
> dual-band AP will be seen by ConnMan as a single network with two BSS, one on
> 2.4GHz and another on 5GHz; does not matter whether they are in the same
> casing or not. Then, the idea would be to combine the capabilities of both
> BSS and doing so the network (Service) will show both capabilities, for
> instance "a" and "g". Is this what you mean? Or on the contrary what you mean
> is that ConnMan will not be able to distinguish to which of the two BSS user
> wants to connect to, "a" or "g" for instance. If it is what you mean, you are
> right, but in any case this is not a very common case where the idea would be
> to leave ConnMan makes the decision based on the strength as it has done so
> far. We don't want to complicate the choice of the user by adding the desired
> band in such
a case.
actually this is a common case these day. Even more common is that you have
access points with 4 radios in it. And in corporate setups you are going to see
the next AP with the same SSID is still in range of the others. That is what
roaming is. There are a few steering IE available, but I doubt that you can
make any reliable assumptions on what technologies a ConnMan service has.
As you noted, the ConnMan service is the WiFi network (identified by SSID).
This could well mean you have an old 2.4 GHz AP with only g and another one
with n technology at home. ConnMan will show your network as Service and not
just a single AP. The concept of a single AP is not exposed by ConnMan.
>>> After that, the only missing information is to know whether "n" and "ac"
>>> are also supported, to do that we could take advantage of the IEs provided
>>> by the wpa_supplicant, there we just would have to check the presence of
>>> the VHT and HT capabilities.
>>> Finally, by combining information from BSSs using the same SSID we would be
>>> able to provide speed information supported by each service: "a", "b", "g",
>>> "n" and "ac".
>>
>> The main question is what are you going to do with it. Since the provided
>> information during scanning are just pure informational. They have nothing
>> to do with the actual data rate that link operates in. And that can change
>> any time and normally it does. Any interference can trigger a change in data
>> rate. Unless the kernel is able to inform us about changes and
>> wpa_supplicant providing them in an asynchronous /event based fashion to
>> ConnMan, there is no point in exposing them. We can not poll wpa_supplicant
>> for updates.
>
> We agreed with you. In fact, we had already done the same analysis, the
> actual data rate computation depends on too many variables (Some of them even
> continually changing) and provide a trusty value is not so easy. That is the
> reason why in the email I only mentioned IEEE standards "a", "b", "g", "n"
> and "ac". I think I used the wrong word from the very beginning, the actual
> data rate is not what we were thinking to provide to the user but the
> supported IEEE standard.
Even that is hard since noted above, ConnMan represents the WiFi network as
Service and not just a single AP. There are no requirements that your WiFi
network is might up of the same IEEE standards. The only real requirement is
that all of them support WPA1 at minimum. Providing false or incomplete
information is worse than providing no information at ll.
>> So I think first of all, link rate changes need to be propagated from the
>> kernel before we consider adding speed information to ConnMan.
>
> After having clarify what we would like to do, we just would like to add that
> the scope of this modification is in the scan/connection phase, where by
> providing this information the user can realize what would be the most
> suitable network he/she should get connected by comparing the IEEE standards
> supported by the available networks in range. Instead, providing the actual
> data rate after user is already connected is not the scope.
You can do this for a connected Service. However first of all you would have to
figure out a way to expose what AP you are connected to. ConnMan does not give
the concept of a single AP to the user. It connects to a WiFi network.
Now this means to make this smooth, you have to put a lot of basic features
into ConnMan so that this makes sense in the term of WiFi network (which is a
ConnMan Service) and its changing connections to APs based on roaming.
And mind you that this also has to work with FullMAC WiFi cards where roaming
is handled in the firmware. So I would start at the kernel and ensure that all
information are send over cfg80211 to identify the AP and its supported
technologies.
Regards
Marcel
------------------------------
Message: 7
Date: Mon, 29 Aug 2016 16:57:38 +0200
From: Jose Blanquicet <[email protected]>
To: Patrik Flykt <[email protected]>, [email protected]
Subject: Re: [PATCH 1/3] technology: Add specific D-Bus methods for
WPS connections
Message-ID:
<cafc8ijldhbylzzxans95hxc40m-+esfwh1uhdqsrm9samqf...@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Hi Patrik,
thanks for your feedback.
>> Two D-Bus methods were added to Technology Interface:
>> - void Start_AP_WPS(string authentication)
>> - void Start_STA_WPS(string authentication)
>
> I'm trying to wrap my brains around the functionality added here. AFAIK
> having these two functions in the technology API makes ConnMan able to
> pass WiFi Alliance testing, right? And you also propose this for
> regular use? But then the precedence between StartSTAWPS() and already
> connected services are not that clear cut.
>
Yes, with these two function ConnMan should be able to get
certificated on WPS by WiFi Alliance.
Yes, the idea is to deprecate the current implementation.
The precedence is explained below.
> Drop '_' from method names.
The idea would be to merge them into a unique one as Tomasz suggested:
StartWPS(type, auth). Where type could be "ap" or "sta". Please look
an important comment regarding this below.
>> With Start_STA_WPS method, WPS was completely detached from Service
>> and now ConnMan follows WiFi Alliance specifications. This patch
>> keeps current WPS implementation for compatibility support, but as
>> deprecated.
>
> As far as I understand the above correctly, the point of this is to
> have ConnMan the STA to "push the WPS button" first, right? And with
> connman requesting WPS, the first AP to also enable WPS will then get
> selected.
Exactly, the main problem of the current implementation is the fact
that user must select the AP to connect to when he/she starts WPS,
which violates the WiFi Alliance specification.
> And the code for (manually) selecting a service which happens to be
> broadcasting/announcing WPS, will automatically of course be connected
> using WPS as before.
Yes, as I mentioned, the patch keeps current WPS implementation but as
deprecated. I didn't duplicate the code, at certain point both
implementations converge to the same wpa_supplicant method call,
therefore they share the code from that point forward.
>> In case of multiple interfaces, Start_STA_WPS will prioritize the
>> STA-only interfaces if they exist, otherwise it will try on STA/AP
>> ones and only as last option it will use STA/AP/P2P interfaces.
>
> If the STA/AP/P2P capability detection works, then yes. To be sure, AP
> interfaces were marked such only after successfully setting up an AP
> for tethering as the capabilities haven't been that perfectly provided
> by wpa_supplicant in the past.
Yes, I used the flag wifi->ap_supported to confirm the AP capabilities
reported by wpa_supplicant. If this flag indicates that AP is not
actually supported then this interface is considered as a STA-only. As
you know this flag is set only after trying to set up an AP for
tethering, but if the user has not tried tethering yet and AP is
present in the capabilities reported by wpa_supplicant then I have
faith on those capabilities and that interface will be taken into
account only if the STA-only interfaces are not available (If they
exist).
>> If the selected interface is already connected to a service or p2p
>> device, it will be disconnected and then the WPS Session will start
>> on it.
>
> Hmm, disconnecting a connected interface sounds a bit unfortunate. Is
> this encouraged by the WiFi Alliance specification? If yes, we might
> not want not to do that, the STA or P2P connection in use can be more
> important for the user than the opposite. From a ConnMan point of view
> it is ok to return an -EBUSY equivalent over D-Bus if everything is
> already in use. Opinions anyone?
No, it is not specified. We just wanted to avoid user to have to
manually make a disconnection. However, for us it is not a problem to
return -EBUSY if it is preferred.
>> Interfaces in AP mode (Tethering enabled) will not be taken into
>> account, they will be skipped.
>
> Good.
>
>> Start_STA_WPS method will finished successfully when WPS-Credentials
>> are correctly received. From that point, all the notifications(error
>> or success) will be sent through the service corresponding to the
>> received credentials.
>
> Shouldn't StartSTAWPS() return the service path on success or how else
> is it known which service got connected via WPS? We do want to provide
> the user with complete information on what the end result was.
Yes, you are right. A "problem" could be if we merge both calls into
an unique one StartWPS(type, auth) as Tomasz suggested, because for
type "STA" the method will return the service path but for "AP" the
return will be an empty string "", is it fine for both of you?
In addition, this patch is based on wpa_supplicant implementation thus
StartSTAWPS() replies successfully when WPS-Provisioning is finished
(Event: WPS Success) not when the whole connection is completely
finished. At this point the WPA2 phase will automatically start using
the passphrase just gotten in the WPS phase and then using the service
path, the user will be able to get all the signals generated in this
second phase on that service, for instance the changes on the "state"
property "association"=>"configuration"=>"ready", as well as the
information related to the IP and the possible errors in this phase.
>> In addition, by using Start_AP_WPS method, ConnMan now supports WPS
>> in AP mode (As registrar) which allows external devices get connect
>> by using WPS. For this method, if tethering is enabled it will start
>> a WPS Session on the corresponding tethering interface, otherwise it
>> will reply with a "PermissionDenied" error or "NotSupported" error if
>> the interface has tethering enabled but it does not support WPS.
>
> This leaves a time window when tethering is enabled using a passphrase,
> but WPS is not active, which was the intention? Else one needs to have
> an attribute for WiFi that can be set before tethering is enabled.
An external STA will be able to get connected at any time when we are
tethering either using the passphrase or WPS (If it has been started).
In fact, in the WPS phase the AP will provide to the external STA the
passphrase to be used, which is the same used to set the tethering in
ConnMan (That is the scope of WPS). Once the external STA gets
connected for the first time it will always use that passphrase for
future re-connections, we tested it. Sorry but what is exactly the
problem here?
>> Technology documentation was updated.
>>
>>
>> ---
>> doc/agent-api.txt | 6 +
>> doc/technology-api.txt | 35 +++++
>
> Although small, these two should go into two different patches as they
> document things.
>> gsupplicant/gsupplicant.h | 15 +++
>> gsupplicant/supplicant.c | 272 +++++++++++++++++++++++++-----------
>> ---
>
> Separate patch for the basic work on the low level supplicant methods.
>> include/technology.h | 10 ++
>> plugins/wifi.c | 322
>> +++++++++++++++++++++++++++++++++++++++++++---
>> src/connman.h | 1 +
>> src/peer.c | 5 +-
>> src/technology.c | 119 +++++++++++++++++
>
>
> From here I'd single out the patches in plugins/wifi.c and src/peer.c
> from the ones in technology. I hope plugins/wifi.c then handles all of
> the WPS STA/peer things for the old and new cases with the same code.
Even if they have no sense on their own? I would try to see if this
patch can be split as following:
1. gsupplicant: Add support for WPS specific methods
(gsupplicant/gsupplicant.h and gsupplicant/supplicant.c)
2. wifi: Add support for WPS specific methods (plugins/wifi.c)
3. peer: Export function to check WPS PIN (src/peer.c and src/connman.h)
4. technology: Add specific WPS D-Bus methods (include/technology.h
and src/technology.c)
5. doc: Add documentation for new WPS D-Bus methods (doc/agent-api.txt
and doc/technology-api.txt)
On the other hand, as I mentioned before the old and the new WPS
implementation converge to the same wpa_supplicant method call, so
they share some pieces of code in plugins/wifi.c. I tried to pay
attention to not duplicate code, as I did for example with the
function for check WPS PIN in src/peer.c.
Regards,
Jose Blanquicet
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 10, Issue 41
***************************************
