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

[Dan Williams <dcbw redhat com>]

On Sun, 2011-05-22 at 22:12 +0100, David Woodhouse wrote:
> On Sat, 2011-05-21 at 10:03 +0300, Pantelis Koukousoulas wrote:
> > 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).
> 
> It would be 'ipv6 ,' because the 'ipv6' option takes a parameter for the
> local and remote interface identifier.
> 
> That just negotiates link-local IPv6 addresses though. To get the
> *global* address (and subnet for routing) you have to do something else.
> That can either be a PPP extension or DHCPv6; I think the latter is
> favoured.
> 
> I can give access to an l2tp connection to someone who seriously wants
> to work on this.
> 
> > 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.
> 
> I talked to Dan on IRC about this; it requires a patch like the one at
> http://bigw.org/~dan/atm-ifindex.patch (which I've now tested and it
> seems fine:
> [root@solos ~]# grep ^ /sys/class/atm/*/atmindex
> /sys/class/atm/solos-pci0/atmindex:  0
> /sys/class/atm/solos-pci1/atmindex:  1
> 
> However, it does have leading whitespace in the 'atmindex' file which I
> wasn't expecting. Was that intentional, Dan?)

Not expected and I cleaned that up before posting just now to netdev@
with some other cleanups.  Please add your tested-by if you don't mind.

> > 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 :)
> 
> You are hard-coding the name 'nas0' for the virtual Ethernet interface,
> which you shouldn't. And I think I'd be tempted not to use the separate
> br2684ctl program; the amount of code you have to 'manage' the external
> tool *far* exceeds the amount of relevant code in br2684ctl.c itself :)

If that's the case maybe we should just do it all internally to NM and
then not have to depend on br2684ctl; though I haven't looked at the
amount of code in there to figure out what's going on.

But yeah, we can't hardcode nas0 in the long run, but how do we get the
kernel to dynamically assign us a device name?  Or do we have to manage
that crap ourselves?

> > >> +             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.
> 
> Hm, OK. Although in PPP you're supposed to *tell* the other side what IP
> address you're using in a ConfReq packet.

Yeah, though how does that work when you don't know what prefix you're
supposed to be using?  I think this breaks down somewhat in IPv6...

Dan