Re: [PATCH net] ipv6: don't auto-add link-local address to lag ports

From: Jay Vosburgh <jay.vosburgh@canonical.com>
To: Jarod Wilson <jarod@redhat.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Moshe Levi <moshele@mellanox.com>,
	Marcelo Ricardo Leitner <mleitner@redhat.com>,
	Netdev <netdev@vger.kernel.org>
Subject: Re: [PATCH net] ipv6: don't auto-add link-local address to lag ports
Date: Wed, 18 Mar 2020 13:41:53 -0700	[thread overview]
Message-ID: <25629.1584564113@famine> (raw)
In-Reply-To: <CAKfmpSc0yea5-OfE1rnVdErDTeOza=owbL00QQEaH-M-A6Za7g@mail.gmail.com>

Jarod Wilson <jarod@redhat.com> wrote:

>On Wed, Mar 18, 2020 at 2:02 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>
>> On 3/18/20 7:06 AM, Jarod Wilson wrote:
>> > Bonding slave and team port devices should not have link-local addresses
>> > automatically added to them, as it can interfere with openvswitch being
>> > able to properly add tc ingress.
>> >
>> > Reported-by: Moshe Levi <moshele@mellanox.com>
>> > CC: Marcelo Ricardo Leitner <mleitner@redhat.com>
>> > CC: netdev@vger.kernel.org
>> > Signed-off-by: Jarod Wilson <jarod@redhat.com>
>>
>>
>> This does not look a net candidate to me, unless the bug has been added recently ?
>>
>> The absence of Fixes: tag is a red flag for a net submission.
>>
>> By adding a Fixes: tag, you are doing us a favor, please.
>
>Yeah, wasn't entirely sure on this one. It fixes a problem, but it's
>not exactly a new one. A quick look at git history suggests this might
>actually be something that technically pre-dates the move to git in
>2005, but only really became a problem with some additional far more
>recent infrastructure (tc and friends). I can resubmit it as net-next
>if that's preferred.

	Commit

c2edacf80e15 bonding / ipv6: no addrconf for slaves separately from master

	should (in theory) already prevent ipv6 link-local addrconf, at
least for bonding slaves, and dates from 2007.  If something has changed
to break the logic in this commit, then (a) you might need to do some
research to find a candidate for your Fixes tag, and (b) I'd suggest
also investigating whether or not the change added by c2edacf80e15 to
addrconf_notify() no longer serves any purpose, and should be removed if
that is the case.

	Note also that the hyperv netvsc driver, in netvsc_vf_join(),
sets IFF_SLAVE in order to trigger the addrconf prevention logic from
c2edacf80e15; I'm not sure if your patch would affect its expectations
(if c2edacf80e15 were removed).

	-J

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