mbox series

[net-next,v2,0/5] vxlan: Join / leave MC group when reconfigured

Message ID cover.1739548836.git.petrm@nvidia.com (mailing list archive)
Headers show
Series vxlan: Join / leave MC group when reconfigured | expand

Message

Petr Machata Feb. 14, 2025, 4:18 p.m. UTC
When a vxlan netdevice is brought up, if its default remote is a multicast
address, the device joins the indicated group.

Therefore when the multicast remote address changes, the device should
leave the current group and subscribe to the new one. Similarly when the
interface used for endpoint communication is changed in a situation when
multicast remote is configured. This is currently not done.

Both vxlan_igmp_join() and vxlan_igmp_leave() can however fail. So it is
possible that with such fix, the netdevice will end up in an inconsistent
situation where the old group is not joined anymore, but joining the
new group fails. Should we join the new group first, and leave the old one
second, we might end up in the opposite situation, where both groups are
joined. Undoing any of this during rollback is going to be similarly
problematic.

One solution would be to just forbid the change when the netdevice is up.
However in vnifilter mode, changing the group address is allowed, and these
problems are simply ignored (see vxlan_vni_update_group()):

 # ip link add name br up type bridge vlan_filtering 1
 # ip link add vx1 up master br type vxlan external vnifilter local 192.0.2.1 dev lo dstport 4789
 # bridge vni add dev vx1 vni 200 group 224.0.0.1
 # tcpdump -i lo &
 # bridge vni add dev vx1 vni 200 group 224.0.0.2
 18:55:46.523438 IP 0.0.0.0 > 224.0.0.22: igmp v3 report, 1 group record(s)
 18:55:46.943447 IP 0.0.0.0 > 224.0.0.22: igmp v3 report, 1 group record(s)
 # bridge vni
 dev               vni                group/remote
 vx1               200                224.0.0.2

Having two different modes of operation for conceptually the same interface
is silly, so in this patchset, just do what the vnifilter code does and
deal with the errors by crossing fingers real hard.

v2:
- Patch #1:
    - New patch.
- Patch #2:
    - Adjust the code so that it is closer to vnifilter.
      Expand the commit message the explain in detail
      which aspects of vnifilter code were emulated.

Petr Machata (5):
  vxlan: Drop 'changelink' parameter from vxlan_dev_configure()
  vxlan: Join / leave MC group after remote changes
  selftests: forwarding: lib: Move require_command to net, generalize
  selftests: test_vxlan_fdb_changelink: Convert to lib.sh
  selftests: test_vxlan_fdb_changelink: Add a test for MC remote change

 drivers/net/vxlan/vxlan_core.c                |  24 +++-
 tools/testing/selftests/net/forwarding/lib.sh |  10 --
 tools/testing/selftests/net/lib.sh            |  19 +++
 .../net/test_vxlan_fdb_changelink.sh          | 111 ++++++++++++++++--
 4 files changed, 136 insertions(+), 28 deletions(-)

Comments

Nikolay Aleksandrov Feb. 16, 2025, 3:10 p.m. UTC | #1
On 2/14/25 18:18, Petr Machata wrote:
> When a vxlan netdevice is brought up, if its default remote is a multicast
> address, the device joins the indicated group.
> 
> Therefore when the multicast remote address changes, the device should
> leave the current group and subscribe to the new one. Similarly when the
> interface used for endpoint communication is changed in a situation when
> multicast remote is configured. This is currently not done.
> 
> Both vxlan_igmp_join() and vxlan_igmp_leave() can however fail. So it is
> possible that with such fix, the netdevice will end up in an inconsistent
> situation where the old group is not joined anymore, but joining the
> new group fails. Should we join the new group first, and leave the old one
> second, we might end up in the opposite situation, where both groups are
> joined. Undoing any of this during rollback is going to be similarly
> problematic.
> 
> One solution would be to just forbid the change when the netdevice is up.
> However in vnifilter mode, changing the group address is allowed, and these
> problems are simply ignored (see vxlan_vni_update_group()):
> 
>  # ip link add name br up type bridge vlan_filtering 1
>  # ip link add vx1 up master br type vxlan external vnifilter local 192.0.2.1 dev lo dstport 4789
>  # bridge vni add dev vx1 vni 200 group 224.0.0.1
>  # tcpdump -i lo &
>  # bridge vni add dev vx1 vni 200 group 224.0.0.2
>  18:55:46.523438 IP 0.0.0.0 > 224.0.0.22: igmp v3 report, 1 group record(s)
>  18:55:46.943447 IP 0.0.0.0 > 224.0.0.22: igmp v3 report, 1 group record(s)
>  # bridge vni
>  dev               vni                group/remote
>  vx1               200                224.0.0.2
> 
> Having two different modes of operation for conceptually the same interface
> is silly, so in this patchset, just do what the vnifilter code does and
> deal with the errors by crossing fingers real hard.
> 
> v2:
> - Patch #1:
>     - New patch.
> - Patch #2:
>     - Adjust the code so that it is closer to vnifilter.
>       Expand the commit message the explain in detail
>       which aspects of vnifilter code were emulated.
> 
> Petr Machata (5):
>   vxlan: Drop 'changelink' parameter from vxlan_dev_configure()
>   vxlan: Join / leave MC group after remote changes
>   selftests: forwarding: lib: Move require_command to net, generalize
>   selftests: test_vxlan_fdb_changelink: Convert to lib.sh
>   selftests: test_vxlan_fdb_changelink: Add a test for MC remote change
> 
>  drivers/net/vxlan/vxlan_core.c                |  24 +++-
>  tools/testing/selftests/net/forwarding/lib.sh |  10 --
>  tools/testing/selftests/net/lib.sh            |  19 +++
>  .../net/test_vxlan_fdb_changelink.sh          | 111 ++++++++++++++++--
>  4 files changed, 136 insertions(+), 28 deletions(-)
> 

For the set:
Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
patchwork-bot+netdevbpf@kernel.org Feb. 18, 2025, 12:10 p.m. UTC | #2
Hello:

This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Fri, 14 Feb 2025 17:18:19 +0100 you wrote:
> When a vxlan netdevice is brought up, if its default remote is a multicast
> address, the device joins the indicated group.
> 
> Therefore when the multicast remote address changes, the device should
> leave the current group and subscribe to the new one. Similarly when the
> interface used for endpoint communication is changed in a situation when
> multicast remote is configured. This is currently not done.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/5] vxlan: Drop 'changelink' parameter from vxlan_dev_configure()
    https://git.kernel.org/netdev/net-next/c/5afb1596b90c
  - [net-next,v2,2/5] vxlan: Join / leave MC group after remote changes
    https://git.kernel.org/netdev/net-next/c/d42d54336834
  - [net-next,v2,3/5] selftests: forwarding: lib: Move require_command to net, generalize
    https://git.kernel.org/netdev/net-next/c/f802f172d78b
  - [net-next,v2,4/5] selftests: test_vxlan_fdb_changelink: Convert to lib.sh
    https://git.kernel.org/netdev/net-next/c/24adf47ea9ac
  - [net-next,v2,5/5] selftests: test_vxlan_fdb_changelink: Add a test for MC remote change
    https://git.kernel.org/netdev/net-next/c/eae1e92a1d41

You are awesome, thank you!