netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jarod Wilson <jarod@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: Dept-GELinuxNICDev@qlogic.com, netdev@vger.kernel.org,
	Manish Chopra <manish.chopra@qlogic.com>
Subject: Re: [PATCH net] net/qlcnic: fix mac address restore in bond mode 5/6
Date: Fri, 06 Nov 2015 11:25:25 -0500	[thread overview]
Message-ID: <563CD475.5090801@redhat.com> (raw)
In-Reply-To: <1446819931-60897-1-git-send-email-jarod@redhat.com>

Jarod Wilson wrote:
> The bonding driver saves a copy of slaves' original mac address and then
> assigns whatever mac as needed to the slave, depending on mode. In at
> least modes 5 and 6 (balance-tlb, balance-alb), it often ends up being the
> mac address of another slave. On release from the bond, the original mac
> address is supposed to get restored via a dev_set_mac_address() call in
> the bonding driver's __bond_release_one() function, which calls the
> slave's ndo_set_mac_address function, which for qlcnic, is
> qlcnic_set_mac().

I didn't entirely flesh out this comment with some other relevant 
thoughts. The qlcnic interface getting assigned another interface's mac 
while in the bond is even more of a problem when you release the qlcnic 
interface and the interface that it was borrowing an address from, 
because now you have two interfaces in the system claiming to have the 
same mac address, which by itself can be problematic, and also causes 
problems if/when you try to add them back into a bond.


> Now, this function tries to be somewhat intelligent and exit early if
> you're trying to set the mac address to the same thing that is already
> set. The problem here is that adapter->mac_addr isn't in sync with
> netdev->dev_addr. The qlcnic driver still has the original mac stored in
> adapter->mac_addr, while the bonding driver has updated netdev->dev_addr,
> so qlcnic thinks we're trying to set the same address it already has.
>
> I think the way to go here, since the function updates both netdev and
> adapter's stored mac addresses, is to check if either of them doesn't
> match the newly requested mac. Simply checking netdev's value only could
> result in a similar mismatch and non-update, so look at both.

-- 
Jarod Wilson
jarod@redhat.com

  reply	other threads:[~2015-11-06 16:25 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-06 14:25 [PATCH net] net/qlcnic: fix mac address restore in bond mode 5/6 Jarod Wilson
2015-11-06 16:25 ` Jarod Wilson [this message]
2015-11-07 18:17 ` David Miller

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=563CD475.5090801@redhat.com \
    --to=jarod@redhat.com \
    --cc=Dept-GELinuxNICDev@qlogic.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=manish.chopra@qlogic.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).