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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4B03949D.4000908@gmail.com>
Date:	Wed, 18 Nov 2009 07:30:53 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Arnd Bergmann <arnd@...db.de>
CC:	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	David Miller <davem@...emloft.net>,
	Stephen Hemminger <shemminger@...tta.com>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	Patrick McHardy <kaber@...sh.net>,
	Patrick Mullaney <pmullaney@...ell.com>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Edge Virtual Bridging <evb@...oogroups.com>,
	Anna Fischer <anna.fischer@...com>,
	bridge@...ts.linux-foundation.org,
	virtualization@...ux-foundation.com,
	Jens Osterkamp <jens@...ux.vnet.ibm.com>,
	Gerhard Stenzel <gerhard.stenzel@...ibm.com>
Subject: Re: [PATCH 1/3] macvlan: Reflect macvlan packets meant for other
 macvlan devices

Arnd Bergmann a écrit :
> From: Eric Biederman <ebiederm@...ssion.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@...ssion.com>
> Signed-off-by: Arnd Bergmann <arnd@...db.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;
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html