On Tue, 01 Aug 2006 11:15:29 +1000
Philip Craig <[EMAIL PROTECTED]> wrote:
> Stephen Hemminger wrote:
> > On Mon, 31 Jul 2006 20:06:41 +1000
> > Philip Craig <[EMAIL PROTECTED]> wrote:
> >
> >> This patch implements transparent ethernet bridging for gre tunnels.
> >> There are a few outstanding issues.
> >
> > Why not use existing bridge code?
>
> It does use the existing bridge code. Perhaps the name is misleading.
> All it does is encapsulate the full ethernet header in a gre packet,
> rather than only layer 3. That is, currently gre uses ARPHRD_IPGRE,
> but bridging requires ARPHRD_ETHER.
>
I am not against making the bridge code smarter to handle other
encapsulation.
> >> Some routers set LLC_SAP_BSPAN in the gre protocol field, and then
> >> give the bpdu packet without any other ethernet/llc header. This patch
> >> currently tries to fake the ethernet/llc header before passing the
> >> packet up, but it is buggy (mac addresses are wrong at least). Maybe a
> >> better approach is to call directly into the bridging code. I didn't try
> >> that at first because it isn't modular, and may break other things that
> >> want to see the packet.
> >
> > Existing bridge code already has spanning tree.
>
> Yes, and I want to use that. But this packet is a bit strange in
> that it does not have the ethernet header on it. So what is the
> best way to pass it to existing code? Either fake the ethernet
> header, or pass it directly?
Likewise if the bridge STP bpdu input code was smarter, it could
deal with it maybe?
>
> >> +#if 0
> >> dev = alloc_netdev(sizeof(*t), name, ipgre_tunnel_setup);
> >> +#else
> >> + dev = alloc_netdev(sizeof(*t), name, ipgre_ether_tunnel_setup);
> >> +#endif
> >
> > "Do, or do not there is no try"
>
> I am looking for comments as to whether adding a netlink interface
> to control this is appropriate.
If we make bridge code type aware, then the ipgre tunnel wouldn't have to
change.
> >> +__be16 ipgre_type_trans(struct sk_buff *skb, int offset)
> >> +{
> >> + u8 *h = skb->data;
> >> + __be16 flags = *(__be16*)h;
> >> + __be16 proto = *(__be16*)(h + 2);
> >> +
> >> + /* WCCP version 1 and 2 protocol decoding.
> >> + * - Change protocol to IP
> >> + * - When dealing with WCCPv2, Skip extra 4 bytes in GRE header
> >> + */
> >> + if (flags == 0 &&
> >> + proto == __constant_htons(ETH_P_WCCP)) {
> >> + proto = __constant_htons(ETH_P_IP);
> >> + if ((*(h + offset) & 0xF0) != 0x40)
> >> + offset += 4;
> >> + }
> >
> > Don't use __constant_htons() except in initializers and switch cases
> > (where gcc is too stupid to optimize the macro).
> >
>
> This is a problem in the existing code, which I am simply moving
> around. Should I fix it at the same time?
Usually if a diff touches some code, I try to make it use current practice.
-
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
