Re: hdlc bridge

From: Krzysztof Halasa
Date: Mon Nov 15 2004 - 19:46:28 EST


Hi,

BTW: I would appreciate being Cc:'ied while discussing generic HDLC
things, especially the ones which are aimed at the official kernels.
Thanks.

Added netdev, too.

Ferenci Daniel <Daniel.Ferenci@xxxxxxxxxxx> writes:

> HDLC bridge is kind of a stack for hdlc bridging.
> It can be usefull for sniffing hdlc traffic (within two hdlc interface).

Right.

Of course, given the isolated nature of such bridge, it can be implemented
in either kernel-space or user-space.

> sethdlc hdlc0 hdlc-bridge
> sethdlc hdlc1 hdlc-bridge

You should be able to use more than one bridge I think - the syntax should
allow this.

Something like:
sethdlc hdlc0 hdlc
sethdlc hdlc1 add-to-bridge hdlc0
or
sethdlc create-hdlc-bridge hdlcbr0
sethdlc hdlc0 add-to-bridge hdlcbr0
sethdlc hdlc1 add-to-bridge hdlcbr0

would probably be better.

> +++ linux-2.4.25.intel/include/net/hdlcbridge.h Sat Nov 13 08:21:12 2004

First, I would do all works against 2.6 kernel. You may want to check
with Marcelo, Jeff and/or David, but I wouldn't expect them to apply
such patches to 2.4 when 2.6 doesn't have them.
I wouldn't do that either.

> +typedef enum {
> + PHYS_OK,
> + PHYS_NOK
> +} STAT;

I don't think such bridge has anything to do with anything "physical".
I would rather change "PHYS" into something related to the actual
nature of underlying device (which could be virtual, a tunnel, or
anything).

> +++ linux-2.4.25.intel/net/Config.in Sat Nov 13 08:22:39 2004
> @@ -1,6 +1,8 @@
> #
> # Network configuration
> #
> +# 12.11.2004 Daniel Ferenci added parts for hdlcbridge
> +
> mainmenu_option next_comment
> comment 'Networking options'
> tristate 'Packet socket' CONFIG_PACKET
> @@ -9,6 +11,9 @@
> fi
> tristate 'Netlink device emulation' CONFIG_NETLINK_DEV
> +tristate 'Netlink sockets' CONFIG_NETLINK
> +

I sense something not related to HDLC bridging here.

> +++ linux-2.4.25.intel/net/hdlcbridge/hdlc_bridge.c Sat Nov 13
...

> +int phys_bridge_data_received(hdlc_device *hdlc, struct sk_buff *skb)

I think it should be run directly in/from hdlc->netif_rx().
Possibly using *_type_trans() etc.
Not sure why it has to be that complicated - one more file in drivers/wan
would be enough, wouldn't it?

> +++ linux-2.4.25.intel/drivers/net/wan/hdlc_bridge.c Sat Nov 13

Exactly such a file.

> +static void bridge_rx(struct sk_buff *skb)
> +{
> + hdlc_device *hdlc = dev_to_hdlc(skb->dev);
> + + printk(KERN_ERR "bridge_rx device: %s", skb->dev->name);
> +
> + if (phys_bridge_data_received(hdlc, skb) == PHYS_OK)
> + return;
> + hdlc->stats.rx_errors++;
> + dev_kfree_skb_any(skb);
> +}

bridge_rx() should IMHO just send the frame to the other end,
if of course possible (device up etc). This module would better know
both ends, right?

> +++ linux-2.4.25.intel/drivers/net/wan/hdlc_generic.c Sat Oct 30

> + case IF_PROTO_LAPB: return hdlc_x25_ioctl(hdlc, ifr);

Something extra I'd say.

> +#include <linux/lapb/ioctl.h>

Same here.

> +#define IF_GET_ST3 0x0003

Hmm... Not sure.

> +#define IF_PROTO_HDLC_BRIDGE 0x200C /* hdlc bridge stack */

This one is probably fine, but not sure about the following two:

> +#define IF_PROTO_HDLC_SEND 0x200D /* hdlc bridge stack */
> +#define IF_PROTO_LAPB 0x200E /* lapb stack */

> -
> + struct lapb_parms_struct *lapb_stuff;

Same here.

> + st3_status *st3;
> + + buffer *data;
> +obj-$(CONFIG_TAHOE9XX) += tahoe9xx.o
> +obj-$(CONFIG_X25TAP) += x25tap.o
> +

Hmm...
--
Krzysztof Halasa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/