[PATCH net-next RESEND] net: bonding: add bond_is_icmpv6_nd() helper

[PATCH net-next RESEND] net: bonding: add bond_is_icmpv6_nd() helper

[Tonghao Zhang]

Introduce ipv6 ns/nd checking helper, using skb_header_pointer()
instead of pskb_network_may_pull() on tx path.

alb_determine_nd introduced from commit 0da8aa00bfcfe

Cc: Jay Vosburgh <jv@jvosburgh.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>
Cc: Nikolay Aleksandrov <razor@blackwall.org>
Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
Signed-off-by: Zengbing Tu <tuzengbing@didiglobal.com>
---
 drivers/net/bonding/bond_alb.c  | 32 +++++++-------------------------
 drivers/net/bonding/bond_main.c | 17 ++---------------
 include/net/bonding.h           | 19 +++++++++++++++++++
 3 files changed, 28 insertions(+), 40 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 2d37b07c8215..8e5b9ce52077 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -19,7 +19,6 @@
 #include <linux/in.h>
 #include <net/arp.h>
 #include <net/ipv6.h>
-#include <net/ndisc.h>
 #include <asm/byteorder.h>
 #include <net/bonding.h>
 #include <net/bond_alb.h>
@@ -1280,27 +1279,6 @@ static int alb_set_mac_address(struct bonding *bond, void *addr)
 	return res;
 }
 
-/* determine if the packet is NA or NS */
-static bool alb_determine_nd(struct sk_buff *skb, struct bonding *bond)
-{
-	struct ipv6hdr *ip6hdr;
-	struct icmp6hdr *hdr;
-
-	if (!pskb_network_may_pull(skb, sizeof(*ip6hdr)))
-		return true;
-
-	ip6hdr = ipv6_hdr(skb);
-	if (ip6hdr->nexthdr != IPPROTO_ICMPV6)
-		return false;
-
-	if (!pskb_network_may_pull(skb, sizeof(*ip6hdr) + sizeof(*hdr)))
-		return true;
-
-	hdr = icmp6_hdr(skb);
-	return hdr->icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT ||
-		hdr->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION;
-}
-
 /************************ exported alb functions ************************/
 
 int bond_alb_initialize(struct bonding *bond, int rlb_enabled)
@@ -1381,7 +1359,7 @@ struct slave *bond_xmit_tlb_slave_get(struct bonding *bond,
 	if (!is_multicast_ether_addr(eth_data->h_dest)) {
 		switch (skb->protocol) {
 		case htons(ETH_P_IPV6):
-			if (alb_determine_nd(skb, bond))
+			if (bond_is_icmpv6_nd(skb))
 				break;
 			fallthrough;
 		case htons(ETH_P_IP):
@@ -1467,16 +1445,20 @@ struct slave *bond_xmit_alb_slave_get(struct bonding *bond,
 			break;
 		}
 
-		if (alb_determine_nd(skb, bond)) {
+		if (bond_is_icmpv6_nd(skb)) {
 			do_tx_balance = false;
 			break;
 		}
 
-		/* The IPv6 header is pulled by alb_determine_nd */
 		/* Additionally, DAD probes should not be tx-balanced as that
 		 * will lead to false positives for duplicate addresses and
 		 * prevent address configuration from working.
 		 */
+		if (!pskb_network_may_pull(skb, sizeof(*ip6hdr))) {
+			do_tx_balance = false;
+			break;
+		}
+
 		ip6hdr = ipv6_hdr(skb);
 		if (ipv6_addr_any(&ip6hdr->saddr)) {
 			do_tx_balance = false;
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 17c7542be6a5..a8034a561011 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5338,10 +5338,6 @@ static bool bond_should_broadcast_neighbor(struct sk_buff *skb,
 					   struct net_device *dev)
 {
 	struct bonding *bond = netdev_priv(dev);
-	struct {
-		struct ipv6hdr ip6;
-		struct icmp6hdr icmp6;
-	} *combined, _combined;
 
 	if (!static_branch_unlikely(&bond_bcast_neigh_enabled))
 		return false;
@@ -5349,19 +5345,10 @@ static bool bond_should_broadcast_neighbor(struct sk_buff *skb,
 	if (!bond->params.broadcast_neighbor)
 		return false;
 
-	if (skb->protocol == htons(ETH_P_ARP))
+	if (skb->protocol == htons(ETH_P_ARP) ||
+	    (skb->protocol == htons(ETH_P_IPV6) && bond_is_icmpv6_nd(skb)))
 		return true;
 
-	if (skb->protocol == htons(ETH_P_IPV6)) {
-		combined = skb_header_pointer(skb, skb_mac_header_len(skb),
-					      sizeof(_combined),
-					      &_combined);
-		if (combined && combined->ip6.nexthdr == NEXTHDR_ICMP &&
-		    (combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION ||
-		     combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT))
-			return true;
-	}
-
 	return false;
 }
 
diff --git a/include/net/bonding.h b/include/net/bonding.h
index e06f0d63b2c1..32d9fcca858c 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -29,6 +29,7 @@
 #include <net/bond_options.h>
 #include <net/ipv6.h>
 #include <net/addrconf.h>
+#include <net/ndisc.h>
 
 #define BOND_MAX_ARP_TARGETS	16
 #define BOND_MAX_NS_TARGETS	BOND_MAX_ARP_TARGETS
@@ -814,4 +815,22 @@ static inline netdev_tx_t bond_tx_drop(struct net_device *dev, struct sk_buff *s
 	return NET_XMIT_DROP;
 }
 
+static inline bool bond_is_icmpv6_nd(struct sk_buff *skb)
+{
+	struct {
+		struct ipv6hdr ip6;
+		struct icmp6hdr icmp6;
+	} *combined, _combined;
+
+	combined = skb_header_pointer(skb, skb_mac_header_len(skb),
+				      sizeof(_combined),
+				      &_combined);
+	if (combined && combined->ip6.nexthdr == NEXTHDR_ICMP &&
+	    (combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION ||
+	     combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT))
+		return true;
+
+	return false;
+}
+
 #endif /* _NET_BONDING_H */
-- 
2.34.1

Re: [PATCH net-next RESEND] net: bonding: add bond_is_icmpv6_nd() helper

[Hangbin Liu]

On Tue, Jul 08, 2025 at 08:32:51PM +0800, Tonghao Zhang wrote:
> Introduce ipv6 ns/nd checking helper, using skb_header_pointer()
> instead of pskb_network_may_pull() on tx path.
> 
> alb_determine_nd introduced from commit 0da8aa00bfcfe
> 
> Cc: Jay Vosburgh <jv@jvosburgh.net>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Simon Horman <horms@kernel.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Andrew Lunn <andrew+netdev@lunn.ch>
> Cc: Nikolay Aleksandrov <razor@blackwall.org>
> Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
> Signed-off-by: Zengbing Tu <tuzengbing@didiglobal.com>
> ---
>  drivers/net/bonding/bond_alb.c  | 32 +++++++-------------------------
>  drivers/net/bonding/bond_main.c | 17 ++---------------
>  include/net/bonding.h           | 19 +++++++++++++++++++
>  3 files changed, 28 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index 2d37b07c8215..8e5b9ce52077 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -19,7 +19,6 @@
>  #include <linux/in.h>
>  #include <net/arp.h>
>  #include <net/ipv6.h>
> -#include <net/ndisc.h>
>  #include <asm/byteorder.h>
>  #include <net/bonding.h>
>  #include <net/bond_alb.h>
> @@ -1280,27 +1279,6 @@ static int alb_set_mac_address(struct bonding *bond, void *addr)
>  	return res;
>  }
>  
> -/* determine if the packet is NA or NS */
> -static bool alb_determine_nd(struct sk_buff *skb, struct bonding *bond)
> -{
> -	struct ipv6hdr *ip6hdr;
> -	struct icmp6hdr *hdr;
> -
> -	if (!pskb_network_may_pull(skb, sizeof(*ip6hdr)))
> -		return true;
> -
> -	ip6hdr = ipv6_hdr(skb);
> -	if (ip6hdr->nexthdr != IPPROTO_ICMPV6)
> -		return false;
> -
> -	if (!pskb_network_may_pull(skb, sizeof(*ip6hdr) + sizeof(*hdr)))
> -		return true;
> -
> -	hdr = icmp6_hdr(skb);
> -	return hdr->icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT ||
> -		hdr->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION;
> -}
> -
>  /************************ exported alb functions ************************/
>  
>  int bond_alb_initialize(struct bonding *bond, int rlb_enabled)
> @@ -1381,7 +1359,7 @@ struct slave *bond_xmit_tlb_slave_get(struct bonding *bond,
>  	if (!is_multicast_ether_addr(eth_data->h_dest)) {
>  		switch (skb->protocol) {
>  		case htons(ETH_P_IPV6):
> -			if (alb_determine_nd(skb, bond))
> +			if (bond_is_icmpv6_nd(skb))
>  				break;
>  			fallthrough;
>  		case htons(ETH_P_IP):
> @@ -1467,16 +1445,20 @@ struct slave *bond_xmit_alb_slave_get(struct bonding *bond,
>  			break;
>  		}
>  
> -		if (alb_determine_nd(skb, bond)) {
> +		if (bond_is_icmpv6_nd(skb)) {
>  			do_tx_balance = false;
>  			break;
>  		}
>  
> -		/* The IPv6 header is pulled by alb_determine_nd */
>  		/* Additionally, DAD probes should not be tx-balanced as that
>  		 * will lead to false positives for duplicate addresses and
>  		 * prevent address configuration from working.
>  		 */
> +		if (!pskb_network_may_pull(skb, sizeof(*ip6hdr))) {
> +			do_tx_balance = false;
> +			break;
> +		}
> +
>  		ip6hdr = ipv6_hdr(skb);
>  		if (ipv6_addr_any(&ip6hdr->saddr)) {
>  			do_tx_balance = false;
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 17c7542be6a5..a8034a561011 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -5338,10 +5338,6 @@ static bool bond_should_broadcast_neighbor(struct sk_buff *skb,
>  					   struct net_device *dev)
>  {
>  	struct bonding *bond = netdev_priv(dev);
> -	struct {
> -		struct ipv6hdr ip6;
> -		struct icmp6hdr icmp6;
> -	} *combined, _combined;
>  
>  	if (!static_branch_unlikely(&bond_bcast_neigh_enabled))
>  		return false;
> @@ -5349,19 +5345,10 @@ static bool bond_should_broadcast_neighbor(struct sk_buff *skb,
>  	if (!bond->params.broadcast_neighbor)
>  		return false;
>  
> -	if (skb->protocol == htons(ETH_P_ARP))
> +	if (skb->protocol == htons(ETH_P_ARP) ||
> +	    (skb->protocol == htons(ETH_P_IPV6) && bond_is_icmpv6_nd(skb)))
>  		return true;
>  
> -	if (skb->protocol == htons(ETH_P_IPV6)) {
> -		combined = skb_header_pointer(skb, skb_mac_header_len(skb),
> -					      sizeof(_combined),
> -					      &_combined);
> -		if (combined && combined->ip6.nexthdr == NEXTHDR_ICMP &&
> -		    (combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION ||
> -		     combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT))
> -			return true;
> -	}
> -
>  	return false;
>  }
>  
> diff --git a/include/net/bonding.h b/include/net/bonding.h
> index e06f0d63b2c1..32d9fcca858c 100644
> --- a/include/net/bonding.h
> +++ b/include/net/bonding.h
> @@ -29,6 +29,7 @@
>  #include <net/bond_options.h>
>  #include <net/ipv6.h>
>  #include <net/addrconf.h>
> +#include <net/ndisc.h>
>  
>  #define BOND_MAX_ARP_TARGETS	16
>  #define BOND_MAX_NS_TARGETS	BOND_MAX_ARP_TARGETS
> @@ -814,4 +815,22 @@ static inline netdev_tx_t bond_tx_drop(struct net_device *dev, struct sk_buff *s
>  	return NET_XMIT_DROP;
>  }
>  
> +static inline bool bond_is_icmpv6_nd(struct sk_buff *skb)
> +{
> +	struct {
> +		struct ipv6hdr ip6;
> +		struct icmp6hdr icmp6;
> +	} *combined, _combined;
> +
> +	combined = skb_header_pointer(skb, skb_mac_header_len(skb),
> +				      sizeof(_combined),
> +				      &_combined);
> +	if (combined && combined->ip6.nexthdr == NEXTHDR_ICMP &&
> +	    (combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION ||
> +	     combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT))
> +		return true;
> +
> +	return false;
> +}
> +
>  #endif /* _NET_BONDING_H */
> -- 
> 2.34.1
> 

Reviewed-by: Hangbin Liu <liuhangbin@gmail.com>

Re: [PATCH net-next RESEND] net: bonding: add bond_is_icmpv6_nd() helper

[Jay Vosburgh]

Tonghao Zhang <tonghao@bamaicloud.com> wrote:

>Introduce ipv6 ns/nd checking helper, using skb_header_pointer()
>instead of pskb_network_may_pull() on tx path.
>
>alb_determine_nd introduced from commit 0da8aa00bfcfe
>
>Cc: Jay Vosburgh <jv@jvosburgh.net>
>Cc: "David S. Miller" <davem@davemloft.net>
>Cc: Eric Dumazet <edumazet@google.com>
>Cc: Jakub Kicinski <kuba@kernel.org>
>Cc: Paolo Abeni <pabeni@redhat.com>
>Cc: Simon Horman <horms@kernel.org>
>Cc: Jonathan Corbet <corbet@lwn.net>
>Cc: Andrew Lunn <andrew+netdev@lunn.ch>
>Cc: Nikolay Aleksandrov <razor@blackwall.org>
>Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
>Signed-off-by: Zengbing Tu <tuzengbing@didiglobal.com>
>---
> drivers/net/bonding/bond_alb.c  | 32 +++++++-------------------------
> drivers/net/bonding/bond_main.c | 17 ++---------------
> include/net/bonding.h           | 19 +++++++++++++++++++
> 3 files changed, 28 insertions(+), 40 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>index 2d37b07c8215..8e5b9ce52077 100644
>--- a/drivers/net/bonding/bond_alb.c
>+++ b/drivers/net/bonding/bond_alb.c
>@@ -19,7 +19,6 @@
> #include <linux/in.h>
> #include <net/arp.h>
> #include <net/ipv6.h>
>-#include <net/ndisc.h>
> #include <asm/byteorder.h>
> #include <net/bonding.h>
> #include <net/bond_alb.h>
>@@ -1280,27 +1279,6 @@ static int alb_set_mac_address(struct bonding *bond, void *addr)
> 	return res;
> }
> 
>-/* determine if the packet is NA or NS */
>-static bool alb_determine_nd(struct sk_buff *skb, struct bonding *bond)
>-{
>-	struct ipv6hdr *ip6hdr;
>-	struct icmp6hdr *hdr;
>-
>-	if (!pskb_network_may_pull(skb, sizeof(*ip6hdr)))
>-		return true;
>-
>-	ip6hdr = ipv6_hdr(skb);
>-	if (ip6hdr->nexthdr != IPPROTO_ICMPV6)
>-		return false;
>-
>-	if (!pskb_network_may_pull(skb, sizeof(*ip6hdr) + sizeof(*hdr)))
>-		return true;
>-
>-	hdr = icmp6_hdr(skb);
>-	return hdr->icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT ||
>-		hdr->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION;
>-}
>-
> /************************ exported alb functions ************************/
> 
> int bond_alb_initialize(struct bonding *bond, int rlb_enabled)
>@@ -1381,7 +1359,7 @@ struct slave *bond_xmit_tlb_slave_get(struct bonding *bond,
> 	if (!is_multicast_ether_addr(eth_data->h_dest)) {
> 		switch (skb->protocol) {
> 		case htons(ETH_P_IPV6):
>-			if (alb_determine_nd(skb, bond))
>+			if (bond_is_icmpv6_nd(skb))
> 				break;
> 			fallthrough;
> 		case htons(ETH_P_IP):
>@@ -1467,16 +1445,20 @@ struct slave *bond_xmit_alb_slave_get(struct bonding *bond,
> 			break;
> 		}
> 
>-		if (alb_determine_nd(skb, bond)) {
>+		if (bond_is_icmpv6_nd(skb)) {
> 			do_tx_balance = false;
> 			break;
> 		}
> 
>-		/* The IPv6 header is pulled by alb_determine_nd */
> 		/* Additionally, DAD probes should not be tx-balanced as that
> 		 * will lead to false positives for duplicate addresses and
> 		 * prevent address configuration from working.
> 		 */
>+		if (!pskb_network_may_pull(skb, sizeof(*ip6hdr))) {
>+			do_tx_balance = false;
>+			break;
>+		}
>+

	It's nice to consolidate some duplicate code and use the more
efficient skb_header_pointer, but the above will do a pull anyway for
nearly every packet that passed through the function.

	Would it be better to not use the bond_is_icmpv6_nd helper here,
but instead call skb_header_pointer directly, and then reuse its
returned data for the hash computation that occurs just after this point
in the code?

> 		ip6hdr = ipv6_hdr(skb);
> 		if (ipv6_addr_any(&ip6hdr->saddr)) {
> 			do_tx_balance = false;
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 17c7542be6a5..a8034a561011 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -5338,10 +5338,6 @@ static bool bond_should_broadcast_neighbor(struct sk_buff *skb,
> 					   struct net_device *dev)
> {
> 	struct bonding *bond = netdev_priv(dev);
>-	struct {
>-		struct ipv6hdr ip6;
>-		struct icmp6hdr icmp6;
>-	} *combined, _combined;
> 
> 	if (!static_branch_unlikely(&bond_bcast_neigh_enabled))
> 		return false;
>@@ -5349,19 +5345,10 @@ static bool bond_should_broadcast_neighbor(struct sk_buff *skb,
> 	if (!bond->params.broadcast_neighbor)
> 		return false;
> 
>-	if (skb->protocol == htons(ETH_P_ARP))
>+	if (skb->protocol == htons(ETH_P_ARP) ||
>+	    (skb->protocol == htons(ETH_P_IPV6) && bond_is_icmpv6_nd(skb)))
> 		return true;
> 
>-	if (skb->protocol == htons(ETH_P_IPV6)) {
>-		combined = skb_header_pointer(skb, skb_mac_header_len(skb),
>-					      sizeof(_combined),
>-					      &_combined);
>-		if (combined && combined->ip6.nexthdr == NEXTHDR_ICMP &&
>-		    (combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION ||
>-		     combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT))
>-			return true;
>-	}
>-
> 	return false;
> }
> 
>diff --git a/include/net/bonding.h b/include/net/bonding.h
>index e06f0d63b2c1..32d9fcca858c 100644
>--- a/include/net/bonding.h
>+++ b/include/net/bonding.h
>@@ -29,6 +29,7 @@
> #include <net/bond_options.h>
> #include <net/ipv6.h>
> #include <net/addrconf.h>
>+#include <net/ndisc.h>
> 
> #define BOND_MAX_ARP_TARGETS	16
> #define BOND_MAX_NS_TARGETS	BOND_MAX_ARP_TARGETS
>@@ -814,4 +815,22 @@ static inline netdev_tx_t bond_tx_drop(struct net_device *dev, struct sk_buff *s
> 	return NET_XMIT_DROP;
> }
> 
>+static inline bool bond_is_icmpv6_nd(struct sk_buff *skb)
>+{

	Minor nit: no need to specify inline, the compiler will decide.

	-J

>+	struct {
>+		struct ipv6hdr ip6;
>+		struct icmp6hdr icmp6;
>+	} *combined, _combined;
>+
>+	combined = skb_header_pointer(skb, skb_mac_header_len(skb),
>+				      sizeof(_combined),
>+				      &_combined);
>+	if (combined && combined->ip6.nexthdr == NEXTHDR_ICMP &&
>+	    (combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION ||
>+	     combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT))
>+		return true;
>+
>+	return false;
>+}
>+
> #endif /* _NET_BONDING_H */
>-- 
>2.34.1
>

---
	-Jay Vosburgh, jv@jvosburgh.net

Re: [PATCH net-next RESEND] net: bonding: add bond_is_icmpv6_nd() helper

[Tonghao Zhang]

> 2025年7月9日 10:04，Jay Vosburgh <jv@jvosburgh.net> 写道：
> 
> Tonghao Zhang <tonghao@bamaicloud.com> wrote:
> 
>> Introduce ipv6 ns/nd checking helper, using skb_header_pointer()
>> instead of pskb_network_may_pull() on tx path.
>> 
>> alb_determine_nd introduced from commit 0da8aa00bfcfe
>> 
>> Cc: Jay Vosburgh <jv@jvosburgh.net>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Eric Dumazet <edumazet@google.com>
>> Cc: Jakub Kicinski <kuba@kernel.org>
>> Cc: Paolo Abeni <pabeni@redhat.com>
>> Cc: Simon Horman <horms@kernel.org>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: Andrew Lunn <andrew+netdev@lunn.ch>
>> Cc: Nikolay Aleksandrov <razor@blackwall.org>
>> Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
>> Signed-off-by: Zengbing Tu <tuzengbing@didiglobal.com>
>> ---
>> drivers/net/bonding/bond_alb.c  | 32 +++++++-------------------------
>> drivers/net/bonding/bond_main.c | 17 ++---------------
>> include/net/bonding.h           | 19 +++++++++++++++++++
>> 3 files changed, 28 insertions(+), 40 deletions(-)
>> 
>> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>> index 2d37b07c8215..8e5b9ce52077 100644
>> --- a/drivers/net/bonding/bond_alb.c
>> +++ b/drivers/net/bonding/bond_alb.c
>> @@ -19,7 +19,6 @@
>> #include <linux/in.h>
>> #include <net/arp.h>
>> #include <net/ipv6.h>
>> -#include <net/ndisc.h>
>> #include <asm/byteorder.h>
>> #include <net/bonding.h>
>> #include <net/bond_alb.h>
>> @@ -1280,27 +1279,6 @@ static int alb_set_mac_address(struct bonding *bond, void *addr)
>> return res;
>> }
>> 
>> -/* determine if the packet is NA or NS */
>> -static bool alb_determine_nd(struct sk_buff *skb, struct bonding *bond)
>> -{
>> - struct ipv6hdr *ip6hdr;
>> - struct icmp6hdr *hdr;
>> -
>> - if (!pskb_network_may_pull(skb, sizeof(*ip6hdr)))
>> - return true;
>> -
>> - ip6hdr = ipv6_hdr(skb);
>> - if (ip6hdr->nexthdr != IPPROTO_ICMPV6)
>> - return false;
>> -
>> - if (!pskb_network_may_pull(skb, sizeof(*ip6hdr) + sizeof(*hdr)))
>> - return true;
>> -
>> - hdr = icmp6_hdr(skb);
>> - return hdr->icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT ||
>> - hdr->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION;
>> -}
>> -
>> /************************ exported alb functions ************************/
>> 
>> int bond_alb_initialize(struct bonding *bond, int rlb_enabled)
>> @@ -1381,7 +1359,7 @@ struct slave *bond_xmit_tlb_slave_get(struct bonding *bond,
>> if (!is_multicast_ether_addr(eth_data->h_dest)) {
>> switch (skb->protocol) {
>> case htons(ETH_P_IPV6):
>> - if (alb_determine_nd(skb, bond))
>> + if (bond_is_icmpv6_nd(skb))
>> break;
>> fallthrough;
>> case htons(ETH_P_IP):
>> @@ -1467,16 +1445,20 @@ struct slave *bond_xmit_alb_slave_get(struct bonding *bond,
>> break;
>> }
>> 
>> - if (alb_determine_nd(skb, bond)) {
>> + if (bond_is_icmpv6_nd(skb)) {
>> do_tx_balance = false;
>> break;
>> }
>> 
>> - /* The IPv6 header is pulled by alb_determine_nd */
>> /* Additionally, DAD probes should not be tx-balanced as that
>>  * will lead to false positives for duplicate addresses and
>>  * prevent address configuration from working.
>>  */
>> + if (!pskb_network_may_pull(skb, sizeof(*ip6hdr))) {
>> + do_tx_balance = false;
>> + break;
>> + }
>> +
> 
> It's nice to consolidate some duplicate code and use the more
> efficient skb_header_pointer, but the above will do a pull anyway for
> nearly every packet that passed through the function.
> 
> Would it be better to not use the bond_is_icmpv6_nd helper here,
> but instead call skb_header_pointer directly, and then reuse its
> returned data for the hash computation that occurs just after this point
> in the code?
sounds good.
> 
>> ip6hdr = ipv6_hdr(skb);
>> if (ipv6_addr_any(&ip6hdr->saddr)) {
>> do_tx_balance = false;
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 17c7542be6a5..a8034a561011 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -5338,10 +5338,6 @@ static bool bond_should_broadcast_neighbor(struct sk_buff *skb,
>>    struct net_device *dev)
>> {
>> struct bonding *bond = netdev_priv(dev);
>> - struct {
>> - struct ipv6hdr ip6;
>> - struct icmp6hdr icmp6;
>> - } *combined, _combined;
>> 
>> if (!static_branch_unlikely(&bond_bcast_neigh_enabled))
>> return false;
>> @@ -5349,19 +5345,10 @@ static bool bond_should_broadcast_neighbor(struct sk_buff *skb,
>> if (!bond->params.broadcast_neighbor)
>> return false;
>> 
>> - if (skb->protocol == htons(ETH_P_ARP))
>> + if (skb->protocol == htons(ETH_P_ARP) ||
>> +     (skb->protocol == htons(ETH_P_IPV6) && bond_is_icmpv6_nd(skb)))
>> return true;
>> 
>> - if (skb->protocol == htons(ETH_P_IPV6)) {
>> - combined = skb_header_pointer(skb, skb_mac_header_len(skb),
>> -       sizeof(_combined),
>> -       &_combined);
>> - if (combined && combined->ip6.nexthdr == NEXTHDR_ICMP &&
>> -     (combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION ||
>> -      combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT))
>> - return true;
>> - }
>> -
>> return false;
>> }
>> 
>> diff --git a/include/net/bonding.h b/include/net/bonding.h
>> index e06f0d63b2c1..32d9fcca858c 100644
>> --- a/include/net/bonding.h
>> +++ b/include/net/bonding.h
>> @@ -29,6 +29,7 @@
>> #include <net/bond_options.h>
>> #include <net/ipv6.h>
>> #include <net/addrconf.h>
>> +#include <net/ndisc.h>
>> 
>> #define BOND_MAX_ARP_TARGETS 16
>> #define BOND_MAX_NS_TARGETS BOND_MAX_ARP_TARGETS
>> @@ -814,4 +815,22 @@ static inline netdev_tx_t bond_tx_drop(struct net_device *dev, struct sk_buff *s
>> return NET_XMIT_DROP;
>> }
>> 
>> +static inline bool bond_is_icmpv6_nd(struct sk_buff *skb)
>> +{
> 
> Minor nit: no need to specify inline, the compiler will decide.
Hi Jay
If we don’t use the inline in the header file, the linker will report an error. I think we should keep the ‘inline’.
ld: drivers/net/bonding/bond_procfs.o: in function `bond_is_icmpv6_nd':
/root/net-next/./include/net/bonding.h:819: multiple definition of `bond_is_icmpv6_nd'; drivers/net/bonding/bond_main.o:/root/net-next/./include/net/bonding.h:819: first defined here
> 
> -J
> 
>> + struct {
>> + struct ipv6hdr ip6;
>> + struct icmp6hdr icmp6;
>> + } *combined, _combined;
>> +
>> + combined = skb_header_pointer(skb, skb_mac_header_len(skb),
>> +       sizeof(_combined),
>> +       &_combined);
>> + if (combined && combined->ip6.nexthdr == NEXTHDR_ICMP &&
>> +     (combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION ||
>> +      combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT))
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> #endif /* _NET_BONDING_H */
>> -- 
>> 2.34.1
>> 
> 
> ---
> -Jay Vosburgh, jv@jvosburgh.net