Re: [PATCH 2/3] ADSL: Carrier Handling and PPP support

[Pantelis Koukousoulas <pktoss gmail com>]

On Sat, May 21, 2011 at 3:22 AM, David Woodhouse <dwmw2 infradead org> wrote:
> Hm, new code in the 21st century without IPv6 support? :(
> I would need that fixed before I could test this.

Well, the reason that I didn't try to implement IPv6 in the first
version is that unfortunately my provider still doesn't support it,
so I wouldn't be able to test if it works :(

Is it true that pretty much the only thing needed for IPv6 is to
add the "ipv6" option to pppd? (plus any networkmanager
specific stuff that may be needed).

If it is like this, it won't be hard to add support so that you can
test the code and maybe also look into adding this to the other
ppp links as well.


>> +             vpi = nm_setting_adsl_get_vpi (adsl_pppoa);
>> +             vci = nm_setting_adsl_get_vci (adsl_pppoa);
>> +             encapsulation = nm_setting_adsl_get_encapsulation (adsl_pppoa);
>> +             vpivci = g_strdup_printf("%s.%s", vpi, vci);
>
> You want to specify device number there, not just assume there's only
> one. There are dual-port PCI ADSL cards that work quite nicely in
> Linux...

I see, you mean like 0.8.35 and 1.8.35 for the second port.
I will try to fix this.

>> +
>> +             nm_cmd_line_add_string (cmd, "plugin");
>> +             nm_cmd_line_add_string (cmd, "pppoatm.so");
>
> Hm, are you also supporting PPPoE(oA)? There are some providers which
> require it. Essentially the *only* thing you do on the ATM link itself
> is run br2684ctl to create a 'virtual' Ethernet device. And then you do
> "normal" PPPoE on that device.

I posted a "proof of concept" implementation of PPPoE patch in this list:
http://www.mail-archive.com/networkmanager-list gnome org/msg17967.html

Could you take a look? This was just enough code to get me a
successful PPPoE connection, but with your help/advise I could
improve it to be more generally useful :)

>> +             nm_cmd_line_add_string (cmd, "noipdefault");
>
> Why's that unconditional? Do we not have the option to set static IP
> addresses on a PPP connection? It's useful in some cases.

My impression is that NetworkManager wants it this way, it gets the IP
that the provider assigns but this doesn't mean it will actually set
this IP, if you say that you want a static IP it will use the one you
provide instead.

>> +     dbus_g_error_domain_register (NM_SETTING_SERIAL_ERROR, NULL, NM_TYPE_SETTING_ADSL_ERROR);
>
> s/SERIAL/ADSL/ ?

Indeed, thanks for spotting :)

Thanks a lot for your interest and your review :)
Pantelis