* [PATCH v10] net: bonding: Add support for IPV6 ns/na to balance-alb/balance-tlb mode
@ 2022-01-25 14:24 Sun Shouxin
2022-01-28 2:47 ` Jakub Kicinski
0 siblings, 1 reply; 6+ messages in thread
From: Sun Shouxin @ 2022-01-25 14:24 UTC (permalink / raw)
To: j.vosburgh, vfalico, andy, davem, kuba
Cc: netdev, linux-kernel, jay.vosburgh, nikolay, huyd12
Since ipv6 neighbor solicitation and advertisement messages
isn't handled gracefully in bond6 driver, we can see packet
drop due to inconsistency between mac address in the option
message and source MAC .
Another examples is ipv6 neighbor solicitation and advertisement
messages from VM via tap attached to host bridge, the src mac
might be changed through balance-alb mode, but it is not synced
with Link-layer address in the option message.
The patch implements bond6's tx handle for ipv6 neighbor
solicitation and advertisement messages.
Suggested-by: Hu Yadi <huyd12@chinatelecom.cn>
Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>
Signed-off-by: Sun Shouxin <sunshouxin@chinatelecom.cn>
---
v9->v10:
- add IPv6 header pull in alb_determine_nd.
- combine bond_xmit_alb_slave_get's IPv6 header
pull with alb_determine_nd's
---
drivers/net/bonding/bond_alb.c | 40 ++++++++++++++++++++++++++++++++--
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 533e476988f2..d9da6eb7f5c2 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1269,6 +1269,37 @@ 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 icmp6hdr *hdr)
+{
+ if (hdr->icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT ||
+ hdr->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION) {
+ return true;
+ }
+
+ return false;
+}
+
+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) {
+ if (!pskb_may_pull(skb, sizeof(*ip6hdr) + sizeof(*hdr)))
+ return true;
+
+ hdr = icmp6_hdr(skb);
+ return __alb_determine_nd(hdr);
+ }
+
+ return false;
+}
+
/************************ exported alb functions ************************/
int bond_alb_initialize(struct bonding *bond, int rlb_enabled)
@@ -1348,8 +1379,11 @@ struct slave *bond_xmit_tlb_slave_get(struct bonding *bond,
/* Do not TX balance any multicast or broadcast */
if (!is_multicast_ether_addr(eth_data->h_dest)) {
switch (skb->protocol) {
- case htons(ETH_P_IP):
case htons(ETH_P_IPV6):
+ if (alb_determine_nd(skb, bond))
+ break;
+ fallthrough;
+ case htons(ETH_P_IP):
hash_index = bond_xmit_hash(bond, skb);
if (bond->params.tlb_dynamic_lb) {
tx_slave = tlb_choose_channel(bond,
@@ -1432,10 +1466,12 @@ struct slave *bond_xmit_alb_slave_get(struct bonding *bond,
break;
}
- if (!pskb_network_may_pull(skb, sizeof(*ip6hdr))) {
+ if (alb_determine_nd(skb, bond)) {
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.
base-commit: dd81e1c7d5fb126e5fbc5c9e334d7b3ec29a16a0
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v10] net: bonding: Add support for IPV6 ns/na to balance-alb/balance-tlb mode
2022-01-25 14:24 [PATCH v10] net: bonding: Add support for IPV6 ns/na to balance-alb/balance-tlb mode Sun Shouxin
@ 2022-01-28 2:47 ` Jakub Kicinski
2022-01-28 11:46 ` 孙守鑫
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2022-01-28 2:47 UTC (permalink / raw)
To: Sun Shouxin
Cc: j.vosburgh, vfalico, andy, davem, netdev, linux-kernel,
jay.vosburgh, nikolay, huyd12
On Tue, 25 Jan 2022 09:24:18 -0500 Sun Shouxin wrote:
> +/* determine if the packet is NA or NS */
> +static bool __alb_determine_nd(struct icmp6hdr *hdr)
> +{
> + if (hdr->icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT ||
> + hdr->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION) {
> + return true;
> + }
> +
> + return false;
> +}
> +
> +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) {
if (ip6hdr->nexthdr != IPPROTO_ICMPV6)
return false;
This way there's no need to indent the rest of the function.
> + if (!pskb_may_pull(skb, sizeof(*ip6hdr) + sizeof(*hdr)))
What happened to the _network part? pskb_network_may_pull(), right?
> + return true;
> +
> + hdr = icmp6_hdr(skb);
> + return __alb_determine_nd(hdr);
Why create a full helper for this condition? Why not just:
return hdr->icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT ||
hdr->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION;
> + }
> +
> + return false;
> +}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v10] net: bonding: Add support for IPV6 ns/na to balance-alb/balance-tlb mode
2022-01-28 2:47 ` Jakub Kicinski
@ 2022-01-28 11:46 ` 孙守鑫
0 siblings, 0 replies; 6+ messages in thread
From: 孙守鑫 @ 2022-01-28 11:46 UTC (permalink / raw)
To: Jakub Kicinski
Cc: j.vosburgh, vfalico, andy, davem, netdev, linux-kernel,
jay.vosburgh, nikolay, huyd12
Thanks your comment, I'll adjust it and send out v11 soon.
在 2022/1/28 10:47, Jakub Kicinski 写道:
> On Tue, 25 Jan 2022 09:24:18 -0500 Sun Shouxin wrote:
>> +/* determine if the packet is NA or NS */
>> +static bool __alb_determine_nd(struct icmp6hdr *hdr)
>> +{
>> + if (hdr->icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT ||
>> + hdr->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION) {
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> +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) {
> if (ip6hdr->nexthdr != IPPROTO_ICMPV6)
> return false;
>
> This way there's no need to indent the rest of the function.
>
>> + if (!pskb_may_pull(skb, sizeof(*ip6hdr) + sizeof(*hdr)))
> What happened to the _network part? pskb_network_may_pull(), right?
>
>> + return true;
>> +
>> + hdr = icmp6_hdr(skb);
>> + return __alb_determine_nd(hdr);
> Why create a full helper for this condition? Why not just:
>
> return hdr->icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT ||
> hdr->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION;
>
>
>> + }
>> +
>> + return false;
>> +}
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v10] net: bonding: Add support for IPV6 ns/na to balance-alb/balance-tlb mode
@ 2022-01-28 2:39 Sun Shouxin
2022-01-28 2:45 ` 孙守鑫
2022-01-28 2:51 ` Jakub Kicinski
0 siblings, 2 replies; 6+ messages in thread
From: Sun Shouxin @ 2022-01-28 2:39 UTC (permalink / raw)
To: j.vosburgh, vfalico, andy, davem, kuba
Cc: netdev, linux-kernel, jay.vosburgh, nikolay, huyd12, sunshouxin
Since ipv6 neighbor solicitation and advertisement messages
isn't handled gracefully in bond6 driver, we can see packet
drop due to inconsistency between mac address in the option
message and source MAC .
Another examples is ipv6 neighbor solicitation and advertisement
messages from VM via tap attached to host bridge, the src mac
might be changed through balance-alb mode, but it is not synced
with Link-layer address in the option message.
The patch implements bond6's tx handle for ipv6 neighbor
solicitation and advertisement messages.
Suggested-by: Hu Yadi <huyd12@chinatelecom.cn>
Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>
Signed-off-by: Sun Shouxin <sunshouxin@chinatelecom.cn>
---
v9->v10:
- add IPv6 header pull in alb_determine_nd.
- combine bond_xmit_alb_slave_get's IPv6 header
pull with alb_determine_nd's
---
drivers/net/bonding/bond_alb.c | 40 ++++++++++++++++++++++++++++++++--
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 533e476988f2..d9da6eb7f5c2 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1269,6 +1269,37 @@ 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 icmp6hdr *hdr)
+{
+ if (hdr->icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT ||
+ hdr->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION) {
+ return true;
+ }
+
+ return false;
+}
+
+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) {
+ if (!pskb_may_pull(skb, sizeof(*ip6hdr) + sizeof(*hdr)))
+ return true;
+
+ hdr = icmp6_hdr(skb);
+ return __alb_determine_nd(hdr);
+ }
+
+ return false;
+}
+
/************************ exported alb functions ************************/
int bond_alb_initialize(struct bonding *bond, int rlb_enabled)
@@ -1348,8 +1379,11 @@ struct slave *bond_xmit_tlb_slave_get(struct bonding *bond,
/* Do not TX balance any multicast or broadcast */
if (!is_multicast_ether_addr(eth_data->h_dest)) {
switch (skb->protocol) {
- case htons(ETH_P_IP):
case htons(ETH_P_IPV6):
+ if (alb_determine_nd(skb, bond))
+ break;
+ fallthrough;
+ case htons(ETH_P_IP):
hash_index = bond_xmit_hash(bond, skb);
if (bond->params.tlb_dynamic_lb) {
tx_slave = tlb_choose_channel(bond,
@@ -1432,10 +1466,12 @@ struct slave *bond_xmit_alb_slave_get(struct bonding *bond,
break;
}
- if (!pskb_network_may_pull(skb, sizeof(*ip6hdr))) {
+ if (alb_determine_nd(skb, bond)) {
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.
base-commit: dd81e1c7d5fb126e5fbc5c9e334d7b3ec29a16a0
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v10] net: bonding: Add support for IPV6 ns/na to balance-alb/balance-tlb mode
2022-01-28 2:39 Sun Shouxin
@ 2022-01-28 2:45 ` 孙守鑫
2022-01-28 2:51 ` Jakub Kicinski
1 sibling, 0 replies; 6+ messages in thread
From: 孙守鑫 @ 2022-01-28 2:45 UTC (permalink / raw)
To: j.vosburgh, vfalico, andy, davem, kuba
Cc: netdev, linux-kernel, jay.vosburgh, nikolay, huyd12,
孙守鑫
Any progress？
在 2022/1/28 10:39, Sun Shouxin 写道:
> Since ipv6 neighbor solicitation and advertisement messages
> isn't handled gracefully in bond6 driver, we can see packet
> drop due to inconsistency between mac address in the option
> message and source MAC .
>
> Another examples is ipv6 neighbor solicitation and advertisement
> messages from VM via tap attached to host bridge, the src mac
> might be changed through balance-alb mode, but it is not synced
> with Link-layer address in the option message.
>
> The patch implements bond6's tx handle for ipv6 neighbor
> solicitation and advertisement messages.
>
> Suggested-by: Hu Yadi <huyd12@chinatelecom.cn>
> Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>
> Signed-off-by: Sun Shouxin <sunshouxin@chinatelecom.cn>
> ---
> v9->v10:
> - add IPv6 header pull in alb_determine_nd.
> - combine bond_xmit_alb_slave_get's IPv6 header
> pull with alb_determine_nd's
> ---
> drivers/net/bonding/bond_alb.c | 40 ++++++++++++++++++++++++++++++++--
> 1 file changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index 533e476988f2..d9da6eb7f5c2 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -1269,6 +1269,37 @@ 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 icmp6hdr *hdr)
> +{
> + if (hdr->icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT ||
> + hdr->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION) {
> + return true;
> + }
> +
> + return false;
> +}
> +
> +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) {
> + if (!pskb_may_pull(skb, sizeof(*ip6hdr) + sizeof(*hdr)))
> + return true;
> +
> + hdr = icmp6_hdr(skb);
> + return __alb_determine_nd(hdr);
> + }
> +
> + return false;
> +}
> +
> /************************ exported alb functions ************************/
>
> int bond_alb_initialize(struct bonding *bond, int rlb_enabled)
> @@ -1348,8 +1379,11 @@ struct slave *bond_xmit_tlb_slave_get(struct bonding *bond,
> /* Do not TX balance any multicast or broadcast */
> if (!is_multicast_ether_addr(eth_data->h_dest)) {
> switch (skb->protocol) {
> - case htons(ETH_P_IP):
> case htons(ETH_P_IPV6):
> + if (alb_determine_nd(skb, bond))
> + break;
> + fallthrough;
> + case htons(ETH_P_IP):
> hash_index = bond_xmit_hash(bond, skb);
> if (bond->params.tlb_dynamic_lb) {
> tx_slave = tlb_choose_channel(bond,
> @@ -1432,10 +1466,12 @@ struct slave *bond_xmit_alb_slave_get(struct bonding *bond,
> break;
> }
>
> - if (!pskb_network_may_pull(skb, sizeof(*ip6hdr))) {
> + if (alb_determine_nd(skb, bond)) {
> 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.
>
> base-commit: dd81e1c7d5fb126e5fbc5c9e334d7b3ec29a16a0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v10] net: bonding: Add support for IPV6 ns/na to balance-alb/balance-tlb mode
2022-01-28 2:39 Sun Shouxin
2022-01-28 2:45 ` 孙守鑫
@ 2022-01-28 2:51 ` Jakub Kicinski
1 sibling, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2022-01-28 2:51 UTC (permalink / raw)
To: Sun Shouxin
Cc: j.vosburgh, vfalico, andy, davem, netdev, linux-kernel,
jay.vosburgh, nikolay, huyd12
On Thu, 27 Jan 2022 21:39:16 -0500 Sun Shouxin wrote:
> Since ipv6 neighbor solicitation and advertisement messages
> isn't handled gracefully in bond6 driver, we can see packet
> drop due to inconsistency between mac address in the option
> message and source MAC .
>
> Another examples is ipv6 neighbor solicitation and advertisement
> messages from VM via tap attached to host bridge, the src mac
> might be changed through balance-alb mode, but it is not synced
> with Link-layer address in the option message.
>
> The patch implements bond6's tx handle for ipv6 neighbor
> solicitation and advertisement messages.
>
> Suggested-by: Hu Yadi <huyd12@chinatelecom.cn>
> Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>
> Signed-off-by: Sun Shouxin <sunshouxin@chinatelecom.cn>
Did you just resend v10? I commented on this patch a few minutes ago...
and it was also a v10. You can see the status of your patch in
patchwork: https://patchwork.kernel.org/project/netdevbpf/list/
There is no need to repost if the patch is in "active" state like "New"
or "Under Review".
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-28 11:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-25 14:24 [PATCH v10] net: bonding: Add support for IPV6 ns/na to balance-alb/balance-tlb mode Sun Shouxin
2022-01-28 2:47 ` Jakub Kicinski
2022-01-28 11:46 ` 孙守鑫
2022-01-28 2:39 Sun Shouxin
2022-01-28 2:45 ` 孙守鑫
2022-01-28 2:51 ` Jakub Kicinski
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).