All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick McHardy <kaber@trash.net>
To: David Miller <davem@davemloft.net>
Cc: shemminger@vyatta.com, netdev@vger.kernel.org
Subject: Re: [RFC] macvlan: proper multicast support
Date: Tue, 05 May 2009 15:15:41 +0200	[thread overview]
Message-ID: <4A003BFD.8050305@trash.net> (raw)
In-Reply-To: <20090427.025634.54725702.davem@davemloft.net>

David Miller wrote:
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Fri, 24 Apr 2009 09:48:08 -0700
> 
>> When using macvlan's multicast packets don't get properly passed between
>> macvlan's which should be logically sharing the network.  Any multicast
>> packet sent on a macvlan should show up lower device and all other macvlan's.
>> Likewise a multicast packet sent on lower device should be received
>> by all macvlan's.
>>
>> The following is one way to do it; build tested only.
>>
>> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> I definitely want to see what Patrick thinks of this.

 From a functional POV the patch looks mostly fine to me, it seems
to simulate the behaviour of a real network accurately to the point
that you could probably create a loop by bridging two macvlans on
the same physical network.

This part looks incorrect however:

>>  static int macvlan_start_xmit(struct sk_buff *skb, struct net_device *dev)
>>  {
>>  	const struct macvlan_dev *vlan = netdev_priv(dev);
>> +	const struct ethhdr *eth = eth_hdr(skb);
>>  	unsigned int len = skb->len;
>>  	int ret;
>>  
>>  	skb->dev = vlan->lowerdev;
>> +	if (is_multicast_ether_addr(eth->h_dest)) {
>> +		macvlan_broadcast(skb, vlan->port, vlan);
>> +		macvlan_clone(skb, vlan->lowerdev);
>> +	}
>> +
>>  	ret = dev_queue_xmit(skb);
>>  

macvlan_clone() will netif_rx() the packet on the lower dev,
which means it will get passed back to the macvlan receive
hook for the same lower device. This will do two things:

- it will deliver the packet to all other macvlan devices on
   the same physical device - which is fine but done twice with
   the manual delivery

- it will deliver the packet to the originating macvlan device,
   which is wrong

The first point is actually a good thing in my opinion, we need
less special handling and ordering of the reception events is
automatically correct.

The delivery to the originating device looks a bit harder, we
don't know the originating macvlan device when the packet is
delivered to the receive handler. I can't think of an easy way
to fix this right now.


  reply	other threads:[~2009-05-05 13:15 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20090423091425.73ed544c@s6510>
     [not found] ` <49F1D5EF.3020306@trash.net>
2009-04-24 16:48   ` [RFC] macvlan: proper multicast support Stephen Hemminger
2009-04-27  9:56     ` David Miller
2009-05-05 13:15       ` Patrick McHardy [this message]
2009-05-05 18:00         ` Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4A003BFD.8050305@trash.net \
    --to=kaber@trash.net \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=shemminger@vyatta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.