Re: [PATCH net-next 2/2] vxlan: allow specifying multiple default destinations

[Mike Rapoport <mike.rapoport@ravellosystems.com>]

On Fri, Apr 26, 2013 at 03:59:49PM +0900, Atzm Watanabe wrote:
> At Thu, 25 Apr 2013 14:03:49 +0300,
> Mike Rapoport wrote:
> > +/* Add remote to default destinations list */
> > +static int vxlan_remote_add(struct vxlan_dev *vxlan, struct nlattr *attr)
> > +{

...

> > +}
> 
> I think the default destinations should be used for not only sending
> but receiving, so when multicast address was added, it should be
> joined to the group, if the interface state is up.
> (Forbidding the change on the running interface may make it easy.)
> 
> Also vxlan_open() and vxlan_stop() will need to control the
> membership of groups in the default destination list.

My original idea was to simulate multicast group with a list of unicast
destinations for cases when multicast is impossible. I'd prefer to limit
the additional destinations for unicast addresses for now. The support
for several multicast groups with the same VNI can be added later on,
IMHO. 
 
> 
> >  /* See if multicast group is already in use by other ID */
> >  static bool vxlan_group_used(struct vxlan_net *vn,
> > @@ -1500,6 +1600,14 @@ static void vxlan_flush(struct vxlan_dev *vxlan)
> >  	spin_unlock_bh(&vxlan->hash_lock);
> >  }
> >  
> > +static void vxlan_remotes_flush(struct vxlan_dev *vxlan)
> > +{
> > +	struct vxlan_rdst *rd;
> > +
> > +	for (rd = vxlan->default_dst.remote_next; rd; rd = rd->remote_next)
> > +		vxlan_remote_destroy(vxlan, rd);
> > +}
> > +
> >  /* Cleanup timer and forwarding table on shutdown */
> >  static int vxlan_stop(struct net_device *dev)
> >  {
> > @@ -1511,6 +1619,7 @@ static int vxlan_stop(struct net_device *dev)
> >  	del_timer_sync(&vxlan->age_timer);
> >  
> >  	vxlan_flush(vxlan);
> > +	vxlan_remotes_flush(vxlan);
> >  
> >  	return 0;
> >  }
> 
> vxlan_stop() is called when interface state changes to down.
> I think the default destinations should not be flushed at this timing,
> and this should be done at dellink instead.

Agree, will fix.

> 
> Thanks.

--
Sincerely yours,
Mike.