netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jay Vosburgh <jay.vosburgh@canonical.com>
To: Jiri Pirko <jiri@resnulli.us>
Cc: Or Gerlitz <ogerlitz@mellanox.com>,
	Veaceslav Falico <vfalico@gmail.com>,
	Eyal Perry <eyalpe@mellanox.com>, netdev <netdev@vger.kernel.org>,
	Noa Osherovich <noaos@mellanox.com>
Subject: Re: bonding directly changes underlying device address
Date: Tue, 13 May 2014 09:30:41 -0700	[thread overview]
Message-ID: <20030.1399998641@localhost.localdomain> (raw)
In-Reply-To: <20140513115527.GC2910@minipsycho.orion>

Jiri Pirko <jiri@resnulli.us> wrote:

>Tue, May 13, 2014 at 01:06:56PM CEST, ogerlitz@mellanox.com wrote:
>>Hi Jay, Veaceslav
>>
>>I see  now that alb_set_slave_mac_addr directly changes the
>>underlying device mac address without calling dev_set_mac_address
>>when running in TLB mode. Is that on purpose? if yes, can you explain
>>why?
>
>I believe that is a bug. dev_addr change should be always done using
>dev_set_mac_address.

	It may or may not be a bug today, but it's done this way on
purpose to work around the limitations of the system when the tlb mode
was implemented.

	The tlb mode wants to have each slave send with source MAC set
to a particular MAC assigned to the slave (which may or may not be the
slave's actual MAC address).  It does not need for slaves other than the
active slave to actually receive any packets (tlb only has load
balancing for TX, RX all goes to one slave), so programming the MAC into
the device was left out on purpose.

	The MAC change was done this way because, when the code was
originally written, many (perhaps most) devices / drivers could not
change their MAC address while open; it was common practice to program
the MAC in the device open function, and nowhere else.  This method
permits the tlb mode to do the MAC juggling on the TX side without
requiring the slave device be able to change its MAC while open (as the
alb mode requires).

>>I suspect this can lead to funny (or actually sad) bugs in networking
>>drivers, can we avoid that?

	Changing this would also remove support for the tlb mode from
any device that cannot change their MAC while open.  I don't know how
many devices that is (it's probably a small number nowadays), but if
it's not zero then an assessment on losing that support has to be made.

	-J

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

  reply	other threads:[~2014-05-13 16:30 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-13 11:06 bonding directly changes underlying device address Or Gerlitz
2014-05-13 11:55 ` Jiri Pirko
2014-05-13 16:30   ` Jay Vosburgh [this message]
2014-05-13 17:38     ` Or Gerlitz
2014-05-14  8:01       ` Veaceslav Falico
2014-05-14  8:08         ` Or Gerlitz
2014-05-15  1:38         ` Jay Vosburgh
2014-05-15  4:55           ` Veaceslav Falico
2014-05-15 12:37             ` Or Gerlitz

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=20030.1399998641@localhost.localdomain \
    --to=jay.vosburgh@canonical.com \
    --cc=eyalpe@mellanox.com \
    --cc=jiri@resnulli.us \
    --cc=netdev@vger.kernel.org \
    --cc=noaos@mellanox.com \
    --cc=ogerlitz@mellanox.com \
    --cc=vfalico@gmail.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 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).