Message ID | 20220125142418.96167-1-sunshouxin@chinatelecom.cn (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [v10] net: bonding: Add support for IPV6 ns/na to balance-alb/balance-tlb mode | expand |
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; > +}
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; >> +}
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.