From: Or Gerlitz <ogerlitz@voltaire.com>
To: Moni Shoua <monis@Voltaire.COM>
Cc: Jay Vosburgh <fubar@us.ibm.com>,
David Miller <davem@davemloft.net>,
Jason Gunthorpe <jgunthorpe@obsidianresearch.com>,
netdev <netdev@vger.kernel.org>,
bonding-devel@lists.sourceforge.net
Subject: Re: [PATCH RESEND] bonding: remap muticast addresses without using dev_close() and dev_open()
Date: Thu, 10 Sep 2009 10:58:23 +0300 [thread overview]
Message-ID: <4AA8B19F.2080704@voltaire.com> (raw)
In-Reply-To: <4AA39E42.9070702@Voltaire.COM>
Moni Shoua wrote:
> This patch fixes commit e36b9d16c6a6d0f59803b3ef04ff3c22c3844c10. The approach there is to call dev_close()/dev_open() whenever the device type is changed in order to remap the device IP multicast addresses to HW multicast addresses. This approach suffers from 2 drawbacks [...] The fix here is to directly remap the IP multicast addresses to HW multicast addresses for a bonding device that changes its type, and nothing else.
Moni,
The approach and patch look good. First, I think it may be more easier
to review and maintain if you separate this to two patches, the first
simply reverting e36b9d16c6a6d0f59803b3ef04ff3c22c3844c10 and the second
the approach suggested by this patch. Second, I think you may be able to
do well with only one event, see next
> @@ -1460,14 +1460,17 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> */
> if (bond->slave_cnt == 0) {
> if (bond_dev->type != slave_dev->type) {
> - dev_close(bond_dev);
> pr_debug("%s: change device type from %d to %d\n",
> bond_dev->name, bond_dev->type, slave_dev->type);
> +
> + netdev_bonding_change(bond_dev, NETDEV_BONDING_OLDTYPE);
> +
> if (slave_dev->type != ARPHRD_ETHER)
> bond_setup_by_slave(bond_dev, slave_dev);
> else
> ether_setup(bond_dev);
> - dev_open(bond_dev);
> +
> + netdev_bonding_change(bond_dev, NETDEV_BONDING_NEWTYPE);
> }
can't you achieve the same impact if just calling
netdev_bonding_change(bond_dev, NETDEV_BONDING_NEWTYPE) after doing the
setup_by_slave, and have the stack call ip_mc_unmap(...) and then
ip_mc_map(...) ???
Or.
next prev parent reply other threads:[~2009-09-10 7:58 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-09-06 11:34 [PATCH RESEND] bonding: remap muticast addresses without using dev_close() and dev_open() Moni Shoua
2009-09-10 7:58 ` Or Gerlitz [this message]
2009-09-10 8:47 ` Moni Shoua
2009-09-11 19:36 ` David Miller
2009-09-14 9:38 ` Moni Shoua
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=4AA8B19F.2080704@voltaire.com \
--to=ogerlitz@voltaire.com \
--cc=bonding-devel@lists.sourceforge.net \
--cc=davem@davemloft.net \
--cc=fubar@us.ibm.com \
--cc=jgunthorpe@obsidianresearch.com \
--cc=monis@Voltaire.COM \
--cc=netdev@vger.kernel.org \
/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.