Hi Guillaume,
>>> - if (ppp_net_set_mtu(ppp->net, ppp->mtu) == FALSE)
>>> - DBG(ppp, "Unable to set MTU");
>>> + /*
>>> + * If we have opened the tun interface locally,
>>> + * we have to set a MTU value.
>>> + */
>> So I don't agree with this. The MTU is negotiated during LCP phase, so
>> unless I'm missing something, this value should still be set to the
>> negotiated one.
>>
>
> Right , I thought that as I was setting a MTU value for ConnMan tun
> interface I didn't need to set the MTU value into oFono.
> Obviously I was wrong :)
>
I assume the MTU is set to the default 1500 in Connman?
>>> + net->if_name = strdup("Server ppp");
>> This looks wrong. You do actually want to return a proper network
>> interface here, not make something up.
>
> In fact, only ConnMan needs to know each name of its interfaces,
> so do we need to fill this field in when ConnMan creates the interface?
>
Remember that we're writing a library, so things must be consistent.
Besides, isn't this is a matter of running a TUNGETIFF ioctl?
>>> + /* create a channel for reading and writing to this
>>> interface */
>>> + channel = g_io_channel_unix_new(fd);
>>> + }
>> There's a small problem of symmetry here. If IPCP is established
>> correctly, then the fd will eventually be closed by ppp_net. However,
>> if we never properly establish IPCP, then fd will not be closed. What
>> is actually expected by ConnMan?
>>
>
> In the case we don't properly establish the IPCP, ppp_disconnect CB of
> emulator will be called.
> Then, we will call the release_private_network DBus method. This method
> will close the fd if it is opened.
>
Again, we're writing a library. We cannot assume some behavior external
to the library is going to save us. So I really suggest we make closing
of the fd symmetric.
Regards,
-Denis
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
