Re: [PATCH 0/3] [RFC] Support for ADSL modems

[Dan Williams <dcbw redhat com>]

On Fri, 2011-05-06 at 14:23 +0300, Pantelis Koukousoulas wrote:
> Hi, before I leave for FOSSCOMM.gr, this patch series implements the initial
> version of the support for ADSL modems in network manager.
> 
> Notes:
>    * PPPoA-only for now. I hope that PPPoE can be added with not too much trouble
>      as well, if it is possible to just create a new ethernet device for the "nas0"
>      interface that br2684ctl creates and reuse the already existing "wired" device
>      PPPoE support.
> 
>      This way the adsl part can manage the ATM part of the equation just like it
>      does now for PPPoA.

Well, the PPPoE bits aren't that complicated, so for now I'd rather
duplicate them for the ADSL code, and we can re-merge it later.  Most of
the code is simply detecting whether PPPoE should be used, and if so,
creating a new NMPPPManager object for the interface, then connecting to
the signals that it provides.  But lets get PPPoA working first and
PPPoE can come later.

>    * This is the "duct tape and gum" implementation, according to the "first make
>      it work, then make it work right" motto. There are lots of bugs/limitations
>      duplicate code etc but at least it works for me (TM) now :P
> 
>    * My initial patch that added everything at once got bounced for moderation
>      due to its size, so (for size and easier reviewing reasons) I split it to
>      3 patches, in a bisect-friendly way (after each patch, NM builds and does
>      something useful)
> 
>    * I 'm now happily waiting for your comments / advice / testing etc :) :)

More comments in reply to the patches themselves...

THanks!
Dan

> Cheers,
> Pantelis
>  
> Pantelis Koukousoulas (3):
>   Settings and initial "scaffolding"
>   Carrier Handling and PPP support
>   Add libnm-glib and nmcli code
> 
>  cli/src/connections.c            |   66 ++++-
>  cli/src/devices.c                |    4 +
>  cli/src/settings.c               |   50 ++++
>  cli/src/settings.h               |    2 +
>  include/NetworkManager.h         |   19 +-
>  introspection/Makefile.am        |    1 +
>  introspection/nm-device-adsl.xml |   15 +
>  libnm-glib/Makefile.am           |    6 +
>  libnm-glib/libnm-glib.ver        |    4 +-
>  libnm-glib/nm-device-adsl.c      |  236 +++++++++++++++
>  libnm-glib/nm-device-adsl.h      |   62 ++++
>  libnm-glib/nm-device.c           |   10 +-
>  libnm-util/Makefile.am           |    2 +
>  libnm-util/libnm-util.ver        |   12 +-
>  libnm-util/nm-connection.c       |   25 ++-
>  libnm-util/nm-connection.h       |    2 +
>  libnm-util/nm-setting-adsl.c     |  476 ++++++++++++++++++++++++++++++
>  libnm-util/nm-setting-adsl.h     |   97 ++++++
>  src/Makefile.am                  |    6 +
>  src/nm-device-adsl.c             |  596 ++++++++++++++++++++++++++++++++++++++
>  src/nm-device-adsl.h             |   60 ++++
>  src/nm-udev-manager.c            |   48 +++-
>  src/ppp-manager/nm-ppp-manager.c |   31 ++-
>  src/settings/nm-settings.c       |    2 +
>  24 files changed, 1804 insertions(+), 28 deletions(-)
>  create mode 100644 introspection/nm-device-adsl.xml
>  create mode 100644 libnm-glib/nm-device-adsl.c
>  create mode 100644 libnm-glib/nm-device-adsl.h
>  create mode 100644 libnm-util/nm-setting-adsl.c
>  create mode 100644 libnm-util/nm-setting-adsl.h
>  create mode 100644 src/nm-device-adsl.c
>  create mode 100644 src/nm-device-adsl.h
>