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

[David Woodhouse <dwmw2 infradead org>]

On Thu, 2011-05-26 at 12:58 -0500, Dan Williams wrote:
> On Sun, 2011-05-22 at 22:12 +0100, David Woodhouse wrote:
> > 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?

As long as we're not using br2684ctl, we can just pass an empty name in
the ATM_NEWBACKENDIF ioctl and the kernel will create a 'nas%d' name for
us automatically.

However, it's not clear how we're then supposed to work out what name it
created for us. Perhaps we need something like...

diff --git a/net/atm/br2684.c b/net/atm/br2684.c
index 2252c20..fd45786 100644
--- a/net/atm/br2684.c
+++ b/net/atm/br2684.c
@@ -652,6 +652,14 @@ static int br2684_create(void __user *arg)
 	if (!netdev)
 		return -ENOMEM;
 
+	if (!ni.ifname[0] &&
+	    copy_to_user (&netdev->name,
+			  arg + offsetof (struct atm_newif_br2684, ifname),
+			  strlen(netdev->name) + 1)) {
+		free_netdev(netdev);
+		return -EFAULT;
+	}
+
 	brdev = BRPRIV(netdev);
 
 	pr_debug("registered netdev %s\n", netdev->name);

Strictly speaking, that's an ABI change for an existing ioctl, and thus
quite naughty. But since I don't think there *are* currently any users
of that ioctl outside br2684ctl, I suspect we'd get away with it...

-- 
David Woodhouse                            Open Source Technology Centre
David Woodhouse intel com                              Intel Corporation