Re: [PATCH 1/3] macvlan: Reflect macvlan packets meant for other macvlan devices

Eric Dumazet
2009-11-18T06:31:12Z

Arnd Bergmann a écrit :
> From: Eric Biederman <ebiederm@xmission.com>
>
> Switch ports do not send packets back out the same port they came
> in on. This causes problems when using a macvlan device inside
> of a network namespace as it becomes impossible to talk to
> other macvlan devices.

This patch is very welcome. I review it and found one oddity.

>
> Signed-off-by: Eric Biederman <ebiederm@xmission.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> +static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> + const struct macvlan_dev *vlan = netdev_priv(dev);
> + const struct macvlan_port *port = vlan->port;
> + const struct macvlan_dev *dest;
> + const struct ethhdr *eth;
>
> - skb->dev = dev;
> - skb->pkt_type = PACKET_HOST;
> + skb->protocol = eth_type_trans(skb, dev);
> + eth = eth_hdr(skb);
>
> - netif_rx(skb);
> - return NULL;
> + skb_dst_drop(skb);

Why do you drop dst here ?

It seems strange, since this driver specifically masks out IFF_XMIT_DST_RELEASE
in its macvlan_setup() :

dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;

If we really want to drop dst, it could be done by caller, if IFF_XMIT_DST_RELEASE
was not masked in macvlan_setup().

> + skb->mark = 0;
> + secpath_reset(skb);
> + nf_reset(skb);
> +
> + if (is_multicast_ether_addr(eth->h_dest)) {
> + macvlan_broadcast(skb, port, dev);
> + return macvlan_xmit_world(skb, dev);
> + }
> +
> + dest = macvlan_hash_lookup(port, eth->h_dest);
> + if (dest)
> + return macvlan_unicast(skb, dest);
> +
> + return macvlan_xmit_world(skb, dev);
> }

# find net drivers/net|xargs grep -n IFF_XMIT_DST_RELEASE
net/8021q/vlan_dev.c:837: dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
net/atm/clip.c:561: dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
net/core/dev.c:1778: if (dev->priv_flags & IFF_XMIT_DST_RELEASE)
net/core/dev.c:5287: dev->priv_flags = IFF_XMIT_DST_RELEASE;
net/ipv4/ip_gre.c:1236: dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
net/ipv4/ipip.c:717: dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
net/ipv6/sit.c:1104: dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
drivers/net/appletalk/ipddp.c:76: dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
drivers/net/bonding/bond_main.c:4534: bond_dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
drivers/net/eql.c:197: dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
drivers/net/ifb.c:162: dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
drivers/net/loopback.c:174: dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
drivers/net/macvlan.c:421: dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
drivers/net/ppp_generic.c:1057: dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
drivers/net/wan/hdlc_fr.c:1057: dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;