David Miller <[EMAIL PROTECTED]> wrote:

>> The same struct vlan_group is assigned to all slave devices and so the only 
>> vlan subinterfaces that exist in this case are the bond<x>.<vlan> 
>> subinterfaces, and the vlan path for both slaves will assign the 
>> bond<x>.<vlan> interface to skb->dev, thereby erasing the information about 
>> where the packet came from.
>
>Assuming it is correct to do the skb_bond() here in the VLAN hwaccel
>RX path, then there is still one piece missing from what I can see.

        I'm not sure it's correct to do everything that skb_bond() does
(I've been away on family business, and I'm a bit rusty coming back to
this), but it is correct to do the "should we drop this packet because
it's a duplicate because bonding is involved" logic (because the VLAN
acceleration processing is indavertently bypassing that step).

>Notice that in the netif_receive_skb() path, the return value from
>skb_bond() is used as the third argument to the deliver_skb() routine
>and friends which in turn gets passed to the packet_type functions.
>
>Bonding, in particular, makes use of this third argument, see:
>
>bond_3ad_lacpdu_recv()

        This function expects the third argument to be the the device
that is actually enslaved to the bond and is acting as the endpoint for
the 802.3ad protocol negotiations.  If it's a VLAN interface under
there, that's what it needs.

>rlb_arp_recv()

        This function doesn't actually use the third argument that I'm
seeing.

>So if the new "orig_dev" you are computing in the VLAN hwaccel RX path
>is the correct one, somehow this has to propagate down to the third
>argument of the packet type ->func() invocations, right?
>
>Finally, I'm still a little stumped about why this change is necessary
>still, to be honest.  When you configure the bond, the slaves should
>be the VLAN devices as far as I can tell.  Therefore it should be the
>"vlan_device->masster" that we are interested in not the top-level
>"dev->master".
>
>If the ethernet is on a VLAN, and the administrator configures the
>underlying ethernet device as the slaves of the bond, this to me seems
>like a misconfiguration rather than something we should put hacks in
>to support.

        Not necessarily.  For a configuration with redundant paths
(either multiple ports or multiple switches) or multiple VLANs over the
same physical ports, it's also plausible to configure the vlan
interfaces above bonding, and make the ethernet devices direct slaves of
bonding.  In that configuration, the vlan interfaces are on top,
followed by the bonding device, and the ethernet devices at the bottom.

        In this case (bond0.555 above bond0 above eth0,eth1,etc),
skb_bond doesn't suppress duplicates because skb_bond is called with the
skb->dev set to the bond0.555 dev, not the ethX dev.  Non-accelerated
VLAN devices don't do this; they'll come in with skb->dev set to ethX
and will go through skb_bond as expected.

        -J

---
        -Jay Vosburgh, IBM Linux Technology Center, [EMAIL PROTECTED]
-
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

Reply via email to