Message ID | 20220125002954.94405-1-sunshouxin@chinatelecom.cn (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [v8] net: bonding: Add support for IPV6 ns/na to balance-alb/balance-tlb mode | expand |
Sun Shouxin <sunshouxin@chinatelecom.cn> 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> >--- > drivers/net/bonding/bond_alb.c | 37 +++++++++++++++++++++++++++++++++- > 1 file changed, 36 insertions(+), 1 deletion(-) > >diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c >index 533e476988f2..d4d8670643e9 100644 >--- a/drivers/net/bonding/bond_alb.c >+++ b/drivers/net/bonding/bond_alb.c >@@ -1269,6 +1269,34 @@ 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; >+ >+ ip6hdr = ipv6_hdr(skb); >+ if (ip6hdr->nexthdr == IPPROTO_ICMPV6) { >+ if (!pskb_may_pull(skb, sizeof(struct ipv6hdr) + sizeof(struct icmp6hdr))) >+ 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 +1376,10 @@ 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; I missed this before, but the new expectation is to have a "fallthrough;" statement when intentionally falling through to the next case. See include/linux/compiler_attributes.h. That nit aside, this still looks fine to me. -J >+ case htons(ETH_P_IP): > hash_index = bond_xmit_hash(bond, skb); > if (bond->params.tlb_dynamic_lb) { > tx_slave = tlb_choose_channel(bond, >@@ -1446,6 +1476,11 @@ struct slave *bond_xmit_alb_slave_get(struct bonding *bond, > break; > } > >+ if (alb_determine_nd(skb, bond)) { >+ do_tx_balance = false; >+ break; >+ } >+ > hash_start = (char *)&ip6hdr->daddr; > hash_size = sizeof(ip6hdr->daddr); > break; > >base-commit: dd81e1c7d5fb126e5fbc5c9e334d7b3ec29a16a0 >-- >2.27.0 > --- -Jay Vosburgh, jay.vosburgh@canonical.com
在 2022/1/25 9:10, Jay Vosburgh 写道: > Sun Shouxin <sunshouxin@chinatelecom.cn> 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> >> --- >> drivers/net/bonding/bond_alb.c | 37 +++++++++++++++++++++++++++++++++- >> 1 file changed, 36 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c >> index 533e476988f2..d4d8670643e9 100644 >> --- a/drivers/net/bonding/bond_alb.c >> +++ b/drivers/net/bonding/bond_alb.c >> @@ -1269,6 +1269,34 @@ 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; >> + >> + ip6hdr = ipv6_hdr(skb); >> + if (ip6hdr->nexthdr == IPPROTO_ICMPV6) { >> + if (!pskb_may_pull(skb, sizeof(struct ipv6hdr) + sizeof(struct icmp6hdr))) >> + 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 +1376,10 @@ 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; > I missed this before, but the new expectation is to have a > "fallthrough;" statement when intentionally falling through to the next > case. See include/linux/compiler_attributes.h. > > That nit aside, this still looks fine to me. > > -J Thanks your comment, I'll adjust it and send out V9 soon. >> + case htons(ETH_P_IP): >> hash_index = bond_xmit_hash(bond, skb); >> if (bond->params.tlb_dynamic_lb) { >> tx_slave = tlb_choose_channel(bond, >> @@ -1446,6 +1476,11 @@ struct slave *bond_xmit_alb_slave_get(struct bonding *bond, >> break; >> } >> >> + if (alb_determine_nd(skb, bond)) { >> + do_tx_balance = false; >> + break; >> + } >> + >> hash_start = (char *)&ip6hdr->daddr; >> hash_size = sizeof(ip6hdr->daddr); >> break; >> >> base-commit: dd81e1c7d5fb126e5fbc5c9e334d7b3ec29a16a0 >> -- >> 2.27.0 >> > --- > -Jay Vosburgh, jay.vosburgh@canonical.com
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c index 533e476988f2..d4d8670643e9 100644 --- a/drivers/net/bonding/bond_alb.c +++ b/drivers/net/bonding/bond_alb.c @@ -1269,6 +1269,34 @@ 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; + + ip6hdr = ipv6_hdr(skb); + if (ip6hdr->nexthdr == IPPROTO_ICMPV6) { + if (!pskb_may_pull(skb, sizeof(struct ipv6hdr) + sizeof(struct icmp6hdr))) + 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 +1376,10 @@ 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; + case htons(ETH_P_IP): hash_index = bond_xmit_hash(bond, skb); if (bond->params.tlb_dynamic_lb) { tx_slave = tlb_choose_channel(bond, @@ -1446,6 +1476,11 @@ struct slave *bond_xmit_alb_slave_get(struct bonding *bond, break; } + if (alb_determine_nd(skb, bond)) { + do_tx_balance = false; + break; + } + hash_start = (char *)&ip6hdr->daddr; hash_size = sizeof(ip6hdr->daddr); break;