Re: [PATCH] EtherIP tunnel driver (RFC 3378)

2006-09-13 Thread Patrick McHardy
There are lots of whitespace errors (trailing whitespace, whitespace
following opening parens, no whitespace after comma, ...) which I'm
going to ignore below, please fix them anyway.

Joerg Roedel wrote:
> diff -uprN -X linux-2.6.17.13/Documentation/dontdiff 
> linux-2.6.17.13-vanilla/net/ipv4/etherip.c linux-2.6.17.13/net/ipv4/etherip.c
> --- linux-2.6.17.13-vanilla/net/ipv4/etherip.c1970-01-01 
> 01:00:00.0 +0100
> +++ linux-2.6.17.13/net/ipv4/etherip.c2006-09-11 21:59:40.0 
> +0200
> @@ -0,0 +1,546 @@
> +/*
> + * etherip.c: Ethernet over IPv4 tunnel driver (according to RFC3378)
> + *
> + * This driver could be used to tunnel Ethernet packets through IPv4
> + * networks. This is especially usefull together with the bridging
> + * code in Linux.
> + *
> + * This code was written with an eye on the IPIP driver in linux from
> + * Sam Lantinga. Thanks for the great work.
> + *
> + *  This program is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU General Public License
> + *  version 2 (no later version) as published by the
> + *  Free Software Foundation.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Joerg Roedel <[EMAIL PROTECTED]>");
> +MODULE_DESCRIPTION("Ethernet over IPv4 tunnel driver");
> +
> +/* 
> + * These 2 defines are taken from ipip.c - if it's good enough for them
> + * it's good enough for me.
> + */
> +#define HASH_SIZE16
> +#define HASH(addr)   ((addr^(addr>>4))&0xF)
> +
> +#define ETHERIP_HEADER   ((u16)0x0300)
> +#define ETHERIP_HLEN 2
> +
> +#define BANNER1 "etherip: Ethernet over IPv4 tunneling driver\n"
> +#define BANNER2 "etherip: (C) 2006 by Joerg Roedel <[EMAIL PROTECTED]>\n"
> +
> +struct etherip_tunnel {
> + struct list_head list;
> + struct net_device *dev;
> + struct net_device_stats stats;
> + struct ip_tunnel_parm parms;
> + unsigned int recursion;
> +};
> +
> +static struct net_device *etherip_tunnel_dev;
> +static struct list_head tunnels[HASH_SIZE];
> +
> +static DEFINE_RWLOCK(etherip_lock);
> +
> +static void etherip_tunnel_setup(struct net_device *dev);
> +
> +/* add a tunnel to the hash */
> +static void etherip_tunnel_add(struct etherip_tunnel *tun)
> +{
> + unsigned h = HASH(tun->parms.iph.daddr);
> + list_add_tail(&tun->list, &tunnels[h]);
> +}
> +
> +/* delete a tunnel from the hash*/
> +static void etherip_tunnel_del(struct etherip_tunnel *tun)
> +{
> + list_del(&tun->list);
> +}
> +
> +/* find a tunnel in the hash by parameters from userspace */
> +static struct etherip_tunnel* etherip_tunnel_find(struct ip_tunnel_parm *p)
> +{
> + struct list_head *ptr;
> + struct etherip_tunnel *ret;
> + unsigned h = HASH(p->iph.daddr);
> +
> + list_for_each(ptr, &tunnels[h]) {
> + ret = list_entry(ptr, struct etherip_tunnel, list);

Again, please use list_for_each_entry

> + if (ret->parms.iph.daddr == p->iph.daddr) {
> + return ret;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +/* find a tunnel by its destination address */
> +static struct etherip_tunnel* etherip_tunnel_locate(u32 remote)
> +{
> + struct list_head *ptr;
> + struct etherip_tunnel *ret;
> + unsigned h = HASH(remote);
> +
> + list_for_each(ptr, &tunnels[h]) {
> + ret = list_entry(ptr, struct etherip_tunnel, list);
> + if (ret->parms.iph.daddr == remote) {
> + return ret;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +/* netdevice start function */
> +static int etherip_tunnel_open(struct net_device *dev)
> +{
> + netif_start_queue(dev);
> + return 0;
> +}
> +
> +/* netdevice stop function */
> +static int etherip_tunnel_stop(struct net_device *dev)
> +{
> + netif_stop_queue(dev);
> + return 0;
> +}
> +
> +/* netdevice hard_start_xmit function
> + * it gets an Ethernet packet in skb and encapsulates it in another IP
> + * packet */
> +static int etherip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> + struct etherip_tunnel *tunnel = netdev_priv(dev);
> + struct rtable *rt;
> + struct iphdr *iph;
> + struct flowi fl;
> + struct net_device *tdev;
> + int max_headroom;
> + struct net_device_stats *stats = &tunnel->stats;
> +
> + if (tunnel->recursion++) {
> + tunnel->stats.collisions++;
> + goto tx_error;
> + }
> +
> + fl.oif = fl.iif  = 0;
> + fl.proto = IPPROTO_ETHERIP;
> + fl.nl_u.ip4_u.daddr  = tunnel->parms.iph.daddr;
> + fl.nl_u.ip4_u.saddr  = tunnel->parms.iph.saddr;
> + fl.nl_u.ip4_u.fwmark = 0;
> + fl.nl_u.ip4_u.tos= 0;

Re: [PATCH] EtherIP tunnel driver (RFC 3378)

2006-09-13 Thread Philip Craig
Joerg Roedel wrote:
> +  To configure tunnels an extra tool is required. You can download
> +  it from http://zlug.fh-zwickau.de/~joro/projects/ under the
> +  EtherIP section. If unsure, say N.

To obtain a list of tunnels, this tool calls SIOCGETTUNNEL
(SIOCDEVPRIVATE + 0) for every device in /proc/net/dev. I don't think
this is safe, but I don't have a solution for you.

Is there a reason why you have a separate tool rather than modifying
iproute2?

I don't know if you are aware of this older etherip patch by Lennert
Buytenhek: http://www.wantstofly.org/~buytenh/etherip/
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] EtherIP tunnel driver (RFC 3378)

2006-09-13 Thread Stephen Hemminger
On Thu, 14 Sep 2006 11:21:22 +1000
Philip Craig <[EMAIL PROTECTED]> wrote:

> Joerg Roedel wrote:
> > +To configure tunnels an extra tool is required. You can download
> > +it from http://zlug.fh-zwickau.de/~joro/projects/ under the
> > +EtherIP section. If unsure, say N.
> 
> To obtain a list of tunnels, this tool calls SIOCGETTUNNEL
> (SIOCDEVPRIVATE + 0) for every device in /proc/net/dev. I don't think
> this is safe, but I don't have a solution for you.
> 
> Is there a reason why you have a separate tool rather than modifying
> iproute2?
> 
> I don't know if you are aware of this older etherip patch by Lennert
> Buytenhek: http://www.wantstofly.org/~buytenh/etherip/

SIOCDEVPRIVATE usage makes it hard to do 32 bit compatibility layer.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] EtherIP tunnel driver (RFC 3378)

2006-09-14 Thread Joerg Roedel
On Thu, Sep 14, 2006 at 11:21:22AM +1000, Philip Craig wrote:
> Joerg Roedel wrote:
> > +To configure tunnels an extra tool is required. You can download
> > +it from http://zlug.fh-zwickau.de/~joro/projects/ under the
> > +EtherIP section. If unsure, say N.
> 
> To obtain a list of tunnels, this tool calls SIOCGETTUNNEL
> (SIOCDEVPRIVATE + 0) for every device in /proc/net/dev. I don't think
> this is safe, but I don't have a solution for you.

You are right. But this is the way the ipip driver does it. In the case
of ipip it is safe, because it is visible as a tunnel interface to
userspace. But my driver registers its devices as Ethernet (it has to,
otherwise the devices will not be usable in a bridge). There is no safe
way to distinguish between real Ethernet devices and devices registered
by my driver. I think about implementing an ioctl to fetch a list of
all EtherIP tunnel devices from the driver.

> Is there a reason why you have a separate tool rather than modifying
> iproute2?

I wrote an own tool for testing. At development I wanted to concentrate
on the driver and not how to modify iproute2. But when the driver
becomes stable and may be included I will add it to iproute2.

> I don't know if you are aware of this older etherip patch by Lennert
> Buytenhek: http://www.wantstofly.org/~buytenh/etherip/

I found this patch after I wrote my own and read the discussions
about it. The patch by Lennert Buytenhek has the same problem of
identifing tunnel devices. Futhermore, his driver handles ICMP and cares
about the payload of the Ethernet packet it transmits (it looks, if the
payload ist IPv4 or IPv6). Further it is configurable to set the DF flag
in outgoing packets. First I think the handling of layer 3 protocols is
beyond the scope of tunnel which transmits layer 2 packets. Second this
behavior may break the transport of non-IP payload in the Ethernet
packets since the Ethernet payload protocol may not know the concept of
a path MTU and needs the full Ethernet MTU of 1500. This is the reason
my driver never sets the DF flag in outgoing packets.

Regards,
Joerg Roedel
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] EtherIP tunnel driver (RFC 3378)

2006-09-14 Thread Patrick McHardy
Joerg Roedel wrote:
> On Thu, Sep 14, 2006 at 11:21:22AM +1000, Philip Craig wrote:
> 
>>Joerg Roedel wrote:
>>
>>>+ To configure tunnels an extra tool is required. You can download
>>>+ it from http://zlug.fh-zwickau.de/~joro/projects/ under the
>>>+ EtherIP section. If unsure, say N.
>>
>>To obtain a list of tunnels, this tool calls SIOCGETTUNNEL
>>(SIOCDEVPRIVATE + 0) for every device in /proc/net/dev. I don't think
>>this is safe, but I don't have a solution for you.
> 
> 
> You are right. But this is the way the ipip driver does it. In the case
> of ipip it is safe, because it is visible as a tunnel interface to
> userspace. But my driver registers its devices as Ethernet (it has to,
> otherwise the devices will not be usable in a bridge). There is no safe
> way to distinguish between real Ethernet devices and devices registered
> by my driver. I think about implementing an ioctl to fetch a list of
> all EtherIP tunnel devices from the driver.


Just do what ipip and gre do, use a network device with a fixed name
for the ioctl (you already have the ethip0 device for this purpose it
appears).

>>Is there a reason why you have a separate tool rather than modifying
>>iproute2?
> 
> 
> I wrote an own tool for testing. At development I wanted to concentrate
> on the driver and not how to modify iproute2. But when the driver
> becomes stable and may be included I will add it to iproute2.


The iproute changes are only a few lines, just add the ethip0 device to
the do_add, do_del, ... commands and set the protocol to IPPROTO_ETHERIP
when an etherip tunnel is requested.

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] EtherIP tunnel driver (RFC 3378)

2006-09-14 Thread Philip Craig
Patrick McHardy wrote:
> Joerg Roedel wrote:
>> On Thu, Sep 14, 2006 at 11:21:22AM +1000, Philip Craig wrote:
>>
>>> Joerg Roedel wrote:
>>>
 +   To configure tunnels an extra tool is required. You can download
 +   it from http://zlug.fh-zwickau.de/~joro/projects/ under the
 +   EtherIP section. If unsure, say N.
>>> To obtain a list of tunnels, this tool calls SIOCGETTUNNEL
>>> (SIOCDEVPRIVATE + 0) for every device in /proc/net/dev. I don't think
>>> this is safe, but I don't have a solution for you.
>>
>> You are right. But this is the way the ipip driver does it. In the case
>> of ipip it is safe, because it is visible as a tunnel interface to
>> userspace. But my driver registers its devices as Ethernet (it has to,
>> otherwise the devices will not be usable in a bridge). There is no safe
>> way to distinguish between real Ethernet devices and devices registered
>> by my driver. I think about implementing an ioctl to fetch a list of
>> all EtherIP tunnel devices from the driver.
> 
> 
> Just do what ipip and gre do, use a network device with a fixed name
> for the ioctl (you already have the ethip0 device for this purpose it
> appears).

That fixed name device isn't used to get a list of tunnels. Instead,
ipip and gre read /proc/net/dev, and check for ARPHRD_TUNNEL or
ARPHRD_IPGRE. This won't work for etherip because it uses ARPHRD_ETHER,
which isn't specific to etherip tunnels. A new ioctl to get a list could
be added (this ioctl would use the fixed name device), is that acceptable?
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] EtherIP tunnel driver (RFC 3378)

2006-09-15 Thread Joerg Roedel
On Fri, Sep 15, 2006 at 09:06:02AM +1000, Philip Craig wrote:
> Patrick McHardy wrote:
> > Joerg Roedel wrote:
> >> On Thu, Sep 14, 2006 at 11:21:22AM +1000, Philip Craig wrote:
> >>
> >>> Joerg Roedel wrote:
> >>>
>  + To configure tunnels an extra tool is required. You can 
>  download
>  + it from http://zlug.fh-zwickau.de/~joro/projects/ under the
>  + EtherIP section. If unsure, say N.
> >>> To obtain a list of tunnels, this tool calls SIOCGETTUNNEL
> >>> (SIOCDEVPRIVATE + 0) for every device in /proc/net/dev. I don't think
> >>> this is safe, but I don't have a solution for you.
> >>
> >> You are right. But this is the way the ipip driver does it. In the case
> >> of ipip it is safe, because it is visible as a tunnel interface to
> >> userspace. But my driver registers its devices as Ethernet (it has to,
> >> otherwise the devices will not be usable in a bridge). There is no safe
> >> way to distinguish between real Ethernet devices and devices registered
> >> by my driver. I think about implementing an ioctl to fetch a list of
> >> all EtherIP tunnel devices from the driver.
> > 
> > 
> > Just do what ipip and gre do, use a network device with a fixed name
> > for the ioctl (you already have the ethip0 device for this purpose it
> > appears).
> 
> That fixed name device isn't used to get a list of tunnels. Instead,
> ipip and gre read /proc/net/dev, and check for ARPHRD_TUNNEL or
> ARPHRD_IPGRE. This won't work for etherip because it uses ARPHRD_ETHER,
> which isn't specific to etherip tunnels. A new ioctl to get a list could
> be added (this ioctl would use the fixed name device), is that acceptable?

The problem is that the ethip0 device also uses ARPHDR_ETHER. The usage
of that device is also unsafe. As I see the situation there are 2
solutions for this problem. First use some other Type identifier for
ethip0. But this is only a quick hack. I think about a new device type
ARPHRD_ETHERIP. This makes the tunnel devices incompatible with the
bridging code. But I think it is possible to convince the bridge code to
accept the special tunnel devices too.
Unfortunately I didn't saw the problem when implementing the driver...

Regards,
Joerg Roedel
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] EtherIP tunnel driver (RFC 3378)

2006-09-18 Thread Lennert Buytenhek
On Mon, Sep 11, 2006 at 10:41:29PM +0200, Joerg Roedel wrote:

> This driver implements the tunneling of Ethernet packets over IPv4
> networks for Linux. It uses the protocol defined in RFC 3378.

Check out the thread "[PATCH][RFC] etherip: Ethernet-in-IPv4 tunneling"
that was on netdev in January of 2005 -- a number of arguments against
etherip (and for tunneling ethernet in GRE) were raised back then.

One of the most significant ones, IMHO:

> Another argument against etherip would be that OpenBSD apparently
> mis-implemented etherip by putting the etherip version nibble in the
> second nibble of the etherip header instead of the first, which would
> probably prevent the linux and OpenBSD versions from interoperating,
> negating the advantage of using etherip in the first place.


cheers,
Lennert
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] EtherIP tunnel driver (RFC 3378)

2006-09-19 Thread Joerg Roedel
On Mon, Sep 18, 2006 at 10:52:52PM +0200, Lennert Buytenhek wrote:

> Check out the thread "[PATCH][RFC] etherip: Ethernet-in-IPv4 tunneling"
> that was on netdev in January of 2005 -- a number of arguments against
> etherip (and for tunneling ethernet in GRE) were raised back then.

I read this thread some weeks ago.  I think there are reasons to have
both variants in the kernel. Since both versions are implemented in
different operatins systems and devices, having both will Linux make
interoperable with all of them.  In fact, the only implementers for
EtherIP I found were various BSD derivates. I actually implemented this
driver upon request of a BSD user who wanted interoperability of the
NetBSD EtherIP implementation with Linux.

> 
> One of the most significant ones, IMHO:
> 
> > Another argument against etherip would be that OpenBSD apparently
> > mis-implemented etherip by putting the etherip version nibble in the
> > second nibble of the etherip header instead of the first, which would
> > probably prevent the linux and OpenBSD versions from interoperating,
> > negating the advantage of using etherip in the first place.

I think this is not really a mistake in the OpenBSD implementation. In
my opinion, the RFC is unclear at this point. I focused on
interoperability in my implementation and did it the same way as OpenBSD
(as NetBSD does also, AFAIK FreeBSD has also an EtherIP implementation,
 but I don't tested it). This is the reason why my driver does not check
the value of the incoming EtherIP header too.

Regards,
Joerg Roedel
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html