Messages in this thread | | | From | Jay Vosburgh <> | Subject | Re: [PATCH net] ipv6: don't auto-add link-local address to lag ports | Date | Thu, 19 Mar 2020 12:52:23 -0700 |
| |
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.
I wonder if this also breaks for the netvsc usage of IFF_SLAVE to suppress ipv6 addrconf? Adding the hyperv maintainers to Cc.
In any event, it looks like addrconf_sysctl_addr_gen_mode() calls addrconf_dev_config() directly, which bypasses the IFF_SLAVE check in addrconf_notify() that would gate other callers.
From my reading, your patch appears to cover a superset of cases as compared to the existing IFF_SLAVE test from c2edacf80e15.
>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.
Can the problem be induced via ip link set ... addrgenmode ? That functionality predates the sysctl interface, looks like it was introduced with
bc91b0f07ada ipv6: addrconf: implement address generation modes
-J
--- -Jay Vosburgh, jay.vosburgh@canonical.com
| |