netdev - Re: [PATCH RFC net-next 0/5] bonding: add IPv6 NS/NA monitor support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <66d6c646-d71d-91d3-993c-fc542bf77e0f@nvidia.com>
Date:   Wed, 26 Jan 2022 13:47:45 +0200
From:   Nikolay Aleksandrov <nikolay@...dia.com>
To:     Hangbin Liu <liuhangbin@...il.com>, <netdev@...r.kernel.org>
CC:     Jay Vosburgh <j.vosburgh@...il.com>,
        Veaceslav Falico <vfalico@...il.com>,
        Andy Gospodarek <andy@...yhouse.net>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        David Ahern <dsahern@...il.com>
Subject: Re: [PATCH RFC net-next 0/5] bonding: add IPv6 NS/NA monitor support

On 26/01/2022 09:35, Hangbin Liu wrote:
> This is an RFC of adding IPv6 NS/NA monitor support for bonding. I
> posted a draft patch before[1]. But that patch is too big and David
> Ahern suggested to split it smaller. So I split the previous patch
> to 5 small ones, maybe not very good :)
> 
> The iproute2 patch is here [2].
> 
> This patch add bond IPv6 NS/NA monitor support. A new option
> ns_ip6_target is added, which is similar with arp_ip_target.
> The IPv6 NS/NA monitor will take effect when there is a valid IPv6
> address. And ARP monitor will stop working.
> 
> A new field struct in6_addr ip6_addr is added to struct bond_opt_value
> for IPv6 support. Thus __bond_opt_init() is also updated to check
> string, addr first.
> 
> Function bond_handle_vlan() is split from bond_arp_send() for both
> IPv4/IPv6 usage.
> 
> To alloc NS message and send out. ndisc_ns_create() and ndisc_send_skb()
> are exported.
> 
> [1] https://lore.kernel.org/netdev/20211124071854.1400032-1-liuhangbin@gmail.com
> [2] https://lore.kernel.org/netdev/20211124071854.1400032-2-liuhangbin@gmail.com
> 
> Hangbin Liu (5):
>   ipv6: separate ndisc_ns_create() from ndisc_send_ns()
>   Bonding: split bond_handle_vlan from bond_arp_send
>   bonding: add ip6_addr for bond_opt_value
>   bonding: add new parameter ns_targets
>   bonding: add new option ns_ip6_target
> 
>  Documentation/networking/bonding.rst |  11 ++
>  drivers/net/bonding/bond_main.c      | 266 ++++++++++++++++++++++++---
>  drivers/net/bonding/bond_netlink.c   |  55 ++++++
>  drivers/net/bonding/bond_options.c   | 142 +++++++++++++-
>  drivers/net/bonding/bond_sysfs.c     |  22 +++
>  include/net/bond_options.h           |  14 +-
>  include/net/bonding.h                |  36 ++++
>  include/net/ndisc.h                  |   5 +
>  include/uapi/linux/if_link.h         |   1 +
>  net/ipv6/ndisc.c                     |  45 +++--
>  tools/include/uapi/linux/if_link.h   |   1 +
>  11 files changed, 549 insertions(+), 49 deletions(-)
> 

Hi,
I'd imagine such option to work alongside ARP, i.e. to be able to have both
ARP and ND targets at the same time. On Rx you can choose which one to check
based on the protocol, at Tx the same. Then you can reuse and extend most of the
current arp procedures to handle IPv6 as well. And most of all remove these ifs
all around the code:
+		if (bond_slave_is_up(slave)) {
+			if (bond_do_ns_validate(bond))
+				bond_ns_send_all(bond, slave);
+			else
+				bond_arp_send_all(bond, slave);
+		}

and just have one procedure that handles both if there are any targets for that protocol.
That will completely remove the need for bond_do_ns_validate() helper.

Also define BOND_MAX_ND_TARGETS as BOND_MAX_ARP_TARGETS just for the namesake.

Another cosmetic nit: adjust for reverse xmas tree ordering of local variables all over.

Cheers,
 Nik