2002-01-31 13:05:17

by Adam J. Richter

[permalink] [raw]
Subject: linux-2.4.3/drivers/net/wan/dscc4.c does not compile


linux-2.4.3/drivers/net/wan/dscc4.c does not compile because
it referes to the undefined data structure "sync_serial_settings".
The changes that cause these problems were newly added in 2.4.3.

Adam J. Richter __ ______________ 4880 Stevens Creek Blvd, Suite 104
[email protected] \ / San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l United States of America
fax +1 408 261-6631 "Free Software For The Rest Of Us."


2002-01-31 17:13:04

by Francois Romieu

[permalink] [raw]
Subject: Re: linux-2.4.3/drivers/net/wan/dscc4.c does not compile

Greetings,

Adam J. Richter <[email protected]> :
>
> linux-2.4.3/drivers/net/wan/dscc4.c does not compile because
`-> 5
> it referes to the undefined data structure "sync_serial_settings".
> The changes that cause these problems were newly added in 2.4.3.

It's known and will be automagically fixed as soon as the new HDLC code
goes in (see message of Krzysztof Halasa on l-k the 2002/01/07).

There's (short) documentation at <URL:http://www.cogenit.fr/dscc4/> for the
brave who wants to play with it. An other patch has been sent to allow the
driver to handle syncppp operation.

Bug the official HDLC maintainer if you want him to push his work. :o)

PS: I read l-k

--
Ueimor

2002-01-31 22:32:19

by Krzysztof Halasa

[permalink] [raw]
Subject: Re: linux-2.4.3/drivers/net/wan/dscc4.c does not compile

Francois Romieu <[email protected]> writes:

> Bug the official HDLC maintainer if you want him to push his work. :o)
>
> PS: I read l-k

BTW: I do too :-)
--
Krzysztof Halasa
Network Administrator

2002-02-02 14:44:50

by Francois Romieu

[permalink] [raw]
Subject: SIOCDEVICE ?

Krzysztof Halasa <[email protected]> :
> Francois Romieu <[email protected]> writes:
>
> > Bug the official HDLC maintainer if you want him to push his work. :o)
> >
> > PS: I read l-k
>
> BTW: I do too :-)

Nice. Let's write then.
Your patch doesn't apply against 2.5.3. I did a quick update and noticed the
patch is the sole user of SIOCDEVICE (with dscc4) and SIOCDEVPRIVATE.

Is there some announce/changelog/heads up I missed ?
Is something supposed to replace both ?

--
Ueimor

2002-02-02 20:44:15

by Jeff Garzik

[permalink] [raw]
Subject: Re: SIOCDEVICE ?

On Sat, Feb 02, 2002 at 03:44:24PM +0100, Francois Romieu wrote:
> Your patch doesn't apply against 2.5.3. I did a quick update and noticed the
> patch is the sole user of SIOCDEVICE (with dscc4) and SIOCDEVPRIVATE.

SIOCDEVPRIVATE is verboten in 2.5.x, it doesn't pass through ioctl
translation layers like that which exists on sparc64 and ia64; they are
untyped awful interfaces.

The correction would perhaps define a real command as needed...

Jeff



2002-02-02 22:19:12

by Francois Romieu

[permalink] [raw]
Subject: Re: SIOCDEVICE ?

Jeff Garzik <[email protected]> :
> On Sat, Feb 02, 2002 at 03:44:24PM +0100, Francois Romieu wrote:
> > Your patch doesn't apply against 2.5.3. I did a quick update and noticed the
> > patch is the sole user of SIOCDEVICE (with dscc4) and SIOCDEVPRIVATE.
>
> SIOCDEVPRIVATE is verboten in 2.5.x, it doesn't pass through ioctl
> translation layers like that which exists on sparc64 and ia64; they are
> untyped awful interfaces.
>
> The correction would perhaps define a real command as needed...

Yes, I've seen the big fat comment in include/linux/sockios.h for
SIOCDEVPRIVATE. I can only infer that SIOCDEVICE isn't allowed any more
as it seems it sneakly escaped from the kernel sources.

<executive summary of Krzysztof Halasa's update>
The struct hdlc_device_struct offers under an union the protocol specific
(raw hdlc, frame relay, cisco, pppsync (1)) parameters of the interface.
Those are set from userspace through ifreq.ifr_settings.data and an
ifreq.ifr_settings.type of IF_PROTO_{HDLC/CISCO/FR/X25},... resp. which
specifies the size of the expected data (2).
You retrieve it from userspace with IF_GET_PROTO.
Once an interface is configured for frame-relay, pvc creation/deletion is
done with IF_PROTO_FR_{ADD/DEL}_PVC.

(1) Let's forget pppsync and it's revolting games with net_device.priv for now.
(2) ifr->ifr_settings.data_length checking duplication should be avoided imho.

</summary>

As this question was postponed until 2.5, I'd like someone to state what the
accepted api will be.

Let's hope it's not too much on-topic. :o)

--
Ueimor

2002-02-02 23:41:36

by Krzysztof Halasa

[permalink] [raw]
Subject: Re: SIOCDEVICE ?

Francois Romieu <[email protected]> writes:

> Your patch doesn't apply against 2.5.3. I did a quick update and noticed the
> patch is the sole user of SIOCDEVICE (with dscc4) and SIOCDEVPRIVATE.

SIOCDEVICE, yes. That's my attempt to create an ioctl interface for
controlling devices. It's defined by the hdlc patch, discussed about
a year (?) ago here. Yes, I think I should post a note here.

SIOCDEVPRIVATE - an older method of controlling net devices. IMHO, not
very useful, could possibly be used as a devel hack. Many net drivers
use it.

A new patch which applies to 2.5.3 is in the usual place:
ftp://ftp.pm.waw.pl/pub/linux/hdlc/
This is what I want included in base kernel.
--
Krzysztof Halasa
Network Administrator

2002-02-02 23:58:07

by Jeff Garzik

[permalink] [raw]
Subject: Re: SIOCDEVICE ?

On Sat, Feb 02, 2002 at 08:14:44PM +0100, Krzysztof Halasa wrote:
> Francois Romieu <[email protected]> writes:
>
> > Your patch doesn't apply against 2.5.3. I did a quick update and noticed the
> > patch is the sole user of SIOCDEVICE (with dscc4) and SIOCDEVPRIVATE.
>
> SIOCDEVICE, yes. That's my attempt to create an ioctl interface for
> controlling devices. It's defined by the hdlc patch, discussed about
> a year (?) ago here. Yes, I think I should post a note here.

This too seems way too generic for including in the kernel.


> A new patch which applies to 2.5.3 is in the usual place:
> ftp://ftp.pm.waw.pl/pub/linux/hdlc/
> This is what I want included in base kernel.

What data is passed through the following structure?

Untyped data has the same problems as I listed for SIOCDEVPRIVATE:

> struct if_settings
> {
> unsigned int type; /* Type of physical device or protocol */
> unsigned int data_length; /* device/protocol data length */
> void * data; /* pointer to data, ignored if length = 0 */
> };

Regards,

Jeff



2002-02-03 00:27:51

by Alan

[permalink] [raw]
Subject: Re: SIOCDEVICE ?

> done with IF_PROTO_FR_{ADD/DEL}_PVC.
>
> (1) Let's forget pppsync and it's revolting games with net_device.priv for now.

The syncppp code needs to be switched to use the generic ppp layer anyway.
It was very much a "we need it now" solution as 2.2 lacked generic ppp

2002-02-03 01:46:02

by Krzysztof Halasa

[permalink] [raw]
Subject: Re: SIOCDEVICE ?

Jeff Garzik <[email protected]> writes:

> The correction would perhaps define a real command as needed...

What about details? You want one ioctl = one command again? I'm confused.

> > SIOCDEVICE, yes. That's my attempt to create an ioctl interface for
> > controlling devices. It's defined by the hdlc patch, discussed about
> > a year (?) ago here. Yes, I think I should post a note here.
>
> This too seems way too generic for including in the kernel.
>
>
> What data is passed through the following structure?
>
> Untyped data has the same problems as I listed for SIOCDEVPRIVATE:
>
> > struct if_settings
> > {
> > unsigned int type; /* Type of physical device or protocol */
> > unsigned int data_length; /* device/protocol data length */
> > void * data; /* pointer to data, ignored if length = 0 */
> > };

It depends on the value of "type", enumerated in include/linux/if.h.
For example, IF_IFACE_V35 uses sync_serial_settings struct, while
IF_PROTO_CISCO uses cisco_proto. The structures are defined in
linux/include/hdlc.h (those related to HDLC protos and sync serial
interfaces of course).

The "data_length" is here for protection - as we have to use structs
of different sizes with different protos etc.


You may think of this as of a union of structs. I don't like real union
as its size would be the size of largest struct (large crypto key
comes to mind).

> It adds undiscussed networking changed which I very much doubt DaveM
> would approve of, and I do not approve of: SIOCDEVICE is far too
> generic for inclusion, and it adds a structure for passing untyped
> data which is very definitely non-portable.

I don't see any non-portable things here (what is it exactly?).

I don't say this is ideal, the requirement of middle ifreq structure
(required by netdev ioctls) isn't the most elegant.

About discussions: last discussion I remember ended with:

struct
{
u16 media_group;
union
{
struct hdlc_physical ...
struct hdlc_bitstream
struct hdlc_protocol
struct fr_protocol
struct eth_physical
struct atm_physical
struct dsl_physical
struct dsl_bitstream
struct tr_physical
struct wireless_physical
struct wireless_80211
struct wireless_auth
} config;
}
(see a thread with message dated 7 Dec 2000 by Alan Cox
(Message-ID <[email protected]>). I think there was
no serious objections, and the SIOCDEVICE is just that, with the union
replaced by individual structs (to save memory and permit future extensions
without breaking binary compatibility), and the whole thing moved to ifreq
to avoid 3rd level of indirection.


>From my point of view, the following ifreq would be better (sort of):

struct ifreq
{
char ifrn_name[IFNAMSIZ]; /* if name, e.g. "en0" */
unsigned int type; /* or media_group */
variable_size_struct defined_by_type;/* not a pointer but real struct*/
};

i.e. something like sockaddr structures. I don't even hope anyone would
accept it.
--
Krzysztof Halasa
Network Administrator

2002-02-03 14:19:43

by Francois Romieu

[permalink] [raw]
Subject: Re: SIOCDEVICE ?

Alan Cox <[email protected]> :
[...]
> The syncppp code needs to be switched to use the generic ppp layer anyway.

Is there someone known working on it or planning to do so during 2.5.x ?

--
Ueimor