linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Jarod Wilson <jarod@redhat.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>,
	Jay Vosburgh <jay.vosburgh@canonical.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: Thu, 19 Mar 2020 15:41:08 -0700	[thread overview]
Message-ID: <20200319154108.2de87e34@hermes.lan> (raw)
In-Reply-To: <CAKfmpScXTnnz6wQK3OZcqw4aM1PaLnBRfQL769JgyR7tgM-u5A@mail.gmail.com>

On Thu, 19 Mar 2020 15:29:51 -0400
Jarod Wilson <jarod@redhat.com> wrote:

> On Thu, Mar 19, 2020 at 1:06 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> > On 3/19/20 9:42 AM, Jarod Wilson wrote:
> >  
> > > Interesting. We'll keep digging over here, but that's definitely not
> > > working for this particular use case with OVS for whatever reason.  
> >
> > I did a quick test and confirmed that my bonding slaves do not have link-local addresses,
> > without anything done to prevent them to appear.
> >
> > You might add a selftest, if you ever find what is the trigger :)  
> 
> Okay, have a basic reproducer, courtesy of Marcelo:
> 
> # ip link add name bond0 type bond
> # ip link set dev ens2f0np0 master bond0
> # ip link set dev ens2f1np2 master bond0
> # ip link set dev bond0 up
> # ip a s
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN
> group default qlen 1000
>     link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>     inet 127.0.0.1/8 scope host lo
>        valid_lft forever preferred_lft forever
>     inet6 ::1/128 scope host
>        valid_lft forever preferred_lft forever
> 2: ens2f0np0: <BROADCAST,MULTICAST,SLAVE,UP,LOWER_UP> mtu 1500 qdisc
> mq master bond0 state UP group default qlen 1000
>     link/ether 00:0f:53:2f:ea:40 brd ff:ff:ff:ff:ff:ff
> 5: ens2f1np2: <NO-CARRIER,BROADCAST,MULTICAST,SLAVE,UP> mtu 1500 qdisc
> mq master bond0 state DOWN group default qlen 1000
>     link/ether 00:0f:53:2f:ea:40 brd ff:ff:ff:ff:ff:ff
> 11: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc
> noqueue state UP group default qlen 1000
>     link/ether 00:0f:53:2f:ea:40 brd ff:ff:ff:ff:ff:ff
>     inet6 fe80::20f:53ff:fe2f:ea40/64 scope link
>        valid_lft forever preferred_lft forever
> 
> (above trimmed to relevant entries, obviously)
> 
> # sysctl net.ipv6.conf.ens2f0np0.addr_gen_mode=0
> net.ipv6.conf.ens2f0np0.addr_gen_mode = 0
> # sysctl net.ipv6.conf.ens2f1np2.addr_gen_mode=0
> net.ipv6.conf.ens2f1np2.addr_gen_mode = 0
> 
> # ip a l ens2f0np0
> 2: ens2f0np0: <BROADCAST,MULTICAST,SLAVE,UP,LOWER_UP> mtu 1500 qdisc
> mq master bond0 state UP group default qlen 1000
>     link/ether 00:0f:53:2f:ea:40 brd ff:ff:ff:ff:ff:ff
>     inet6 fe80::20f:53ff:fe2f:ea40/64 scope link tentative
>        valid_lft forever preferred_lft forever
> # ip a l ens2f1np2
> 5: ens2f1np2: <NO-CARRIER,BROADCAST,MULTICAST,SLAVE,UP> mtu 1500 qdisc
> mq master bond0 state DOWN group default qlen 1000
>     link/ether 00:0f:53:2f:ea:40 brd ff:ff:ff:ff:ff:ff
>     inet6 fe80::20f:53ff:fe2f:ea40/64 scope link tentative
>        valid_lft forever preferred_lft forever
> 
> Looks like addrconf_sysctl_addr_gen_mode() bypasses the original "is
> this a slave interface?" check, and results in an address getting
> added, while w/the proposed patch added, no address gets added.
> 
> Looking back through git history again, I see a bunch of 'Fixes:
> d35a00b8e33d ("net/ipv6: allow sysctl to change link-local address
> generation mode")' patches, and I guess that's where this issue was
> also introduced.
> 

Yes the addrgen mode patches caused bad things to happen with hyper-v
sub devices.  Addrconf code is very tricky to get right.
If you look back there have been a large number of changes where
a patch looks good, gets reviewed, merged, and then breaks something
and has to be reverted.

Probably the original patch should just be reverted rather than
trying to add more here.

  parent reply	other threads:[~2020-03-19 22:41 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-18 14:06 [PATCH net] ipv6: don't auto-add link-local address to lag ports Jarod Wilson
2020-03-18 18:02 ` Eric Dumazet
2020-03-18 18:32   ` Jarod Wilson
2020-03-18 20:41     ` Jay Vosburgh
2020-03-19 16:42       ` Jarod Wilson
2020-03-19 17:05         ` Eric Dumazet
2020-03-19 19:29           ` Jarod Wilson
2020-03-19 19:52             ` Jay Vosburgh
2020-03-19 21:53               ` Jarod Wilson
2020-03-19 22:41             ` Stephen Hemminger [this message]
2020-03-23 17:25               ` Jarod Wilson
2020-03-30 15:22                 ` [PATCH net v2] " Jarod Wilson
2020-04-01 18:14                   ` 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=20200319154108.2de87e34@hermes.lan \
    --to=stephen@networkplumber.org \
    --cc=eric.dumazet@gmail.com \
    --cc=jarod@redhat.com \
    --cc=jay.vosburgh@canonical.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mleitner@redhat.com \
    --cc=moshele@mellanox.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).