ip6_route_input() attaches the input route to the skb without taking a reference, relying on the RCU read-side section the lookup runs in. The seg6local input actions also look the route up and hand the skb to dst_input() within one RCU read-side section: the one of the receive path, or that of the nfnetlink verdict handler for a packet reinjected from NFQUEUE. Yet seg6_lookup_any_nexthop() takes and drops a route reference for every packet. Give seg6_lookup_any_nexthop() a noref argument, and set it for the input and table lookups of those actions, as ip6_route_input() does. A packet that races with the release of its route can then be dropped without an ICMPv6 error, where ip6_hold_safe() used to send it on to the next rule or to ip6_null_entry; ip6_route_input() has the same window. seg6_lookup_nexthop(), which the BPF LWT helpers also call, keeps the refcounted lookup: BPF_PROG_TEST_RUN runs them repeatedly on the same skb and may leave the RCU read-side section between runs. Throughput at 0.5% packet loss of 94-byte End packets (two-segment SRH, no payload), spread by RSS on an ixgbe 82599ES across two 2.30 GHz Xeon E5-2650 v3 with 10 cores each, offered by TRex and binary-searched over 10 runs of 10 s, in Mpps: queues cores before after 1 1 0.740 0.756 (+2.2%) 10 10 on the NIC's socket 6.404 6.950 (+8.5%) 16 10 + 6 on two sockets 6.550 10.124 (+54.6%) With the default rules, fib6_rule_lookup() tries the local table first, and the miss takes and drops a reference on ip6_null_entry, one route shared by every CPU. At 16 queues that cache line bounces between the sockets, and ip6_hold_safe() and dst_release() together take about a third of the cycles. Signed-off-by: Yuya Kusakabe --- net/ipv6/seg6_local.c | 55 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c index 0c3d1be7f129..7fbcf590297d 100644 --- a/net/ipv6/seg6_local.c +++ b/net/ipv6/seg6_local.c @@ -319,7 +319,7 @@ static struct ipv6_sr_hdr *advance_nextseg(struct sk_buff *skb, static int seg6_lookup_any_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr, - u32 tbl_id, bool local_delivery, int oif) + u32 tbl_id, bool local_delivery, int oif, bool noref) { struct net *net = dev_net(skb->dev); struct ipv6hdr *hdr = ipv6_hdr(skb); @@ -342,10 +342,14 @@ seg6_lookup_any_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr, fl6.flowi6_flags = FLOWI_FLAG_KNOWN_NH; if (!tbl_id && !oif) { + if (noref) + flags |= RT6_LOOKUP_F_DST_NOREF; dst = ip6_route_input_lookup(net, skb->dev, &fl6, skb, flags); } else if (tbl_id) { struct fib6_table *table; + if (noref) + flags |= RT6_LOOKUP_F_DST_NOREF; table = fib6_get_table(net, tbl_id); if (!table) goto out; @@ -363,26 +367,45 @@ seg6_lookup_any_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr, dev_flags |= IFF_LOOPBACK; if (dst && (dst_dev(dst)->flags & dev_flags) && !dst->error) { - dst_release(dst); + ip6_rt_put_flags(dst_rt6_info(dst), flags); dst = NULL; } out: if (!dst) { - rt = net->ipv6.ip6_blk_hole_entry; - dst = &rt->dst; - dst_hold(dst); + dst = &net->ipv6.ip6_blk_hole_entry->dst; + if (!(flags & RT6_LOOKUP_F_DST_NOREF)) + dst_hold(dst); } skb_dst_drop(skb); - skb_dst_set(skb, dst); + if ((flags & RT6_LOOKUP_F_DST_NOREF) && !dst->rt_uncached_list) + skb_dst_set_noref(skb, dst); + else + skb_dst_set(skb, dst); return dst->error; } +/* rcu must be held from the lookup until dst_input() */ +static int +seg6_lookup_any_nexthop_noref(struct sk_buff *skb, struct in6_addr *nhaddr, + u32 tbl_id, bool local_delivery, int oif) +{ + return seg6_lookup_any_nexthop(skb, nhaddr, tbl_id, local_delivery, oif, + true); +} + +/* rcu must be held from the lookup until dst_input() */ +static int seg6_lookup_nexthop_noref(struct sk_buff *skb, + struct in6_addr *nhaddr, u32 tbl_id) +{ + return seg6_lookup_any_nexthop_noref(skb, nhaddr, tbl_id, false, 0); +} + int seg6_lookup_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr, u32 tbl_id) { - return seg6_lookup_any_nexthop(skb, nhaddr, tbl_id, false, 0); + return seg6_lookup_any_nexthop(skb, nhaddr, tbl_id, false, 0, false); } static __u8 seg6_flv_lcblock_octects(const struct seg6_flavors_info *finfo) @@ -446,7 +469,7 @@ seg6_next_csid_advance_arg(struct sk_buff *skb, static int input_action_end_finish(struct sk_buff *skb, struct seg6_local_lwt *slwt) { - seg6_lookup_nexthop(skb, NULL, 0); + seg6_lookup_nexthop_noref(skb, NULL, 0); return dst_input(skb); } @@ -497,7 +520,7 @@ static int end_next_csid_core(struct sk_buff *skb, struct seg6_local_lwt *slwt) static int input_action_end_x_finish(struct sk_buff *skb, struct seg6_local_lwt *slwt) { - seg6_lookup_any_nexthop(skb, &slwt->nh6, 0, false, slwt->oif); + seg6_lookup_any_nexthop_noref(skb, &slwt->nh6, 0, false, slwt->oif); return dst_input(skb); } @@ -939,7 +962,7 @@ static int input_action_end_t(struct sk_buff *skb, struct seg6_local_lwt *slwt) goto drop; } - seg6_lookup_nexthop(skb, NULL, slwt->table); + seg6_lookup_nexthop_noref(skb, NULL, slwt->table); return dst_input(skb); @@ -1023,7 +1046,7 @@ static int input_action_end_dx6_finish(struct net *net, struct sock *sk, if (!ipv6_addr_any(&slwt->nh6)) nhaddr = &slwt->nh6; - seg6_lookup_nexthop(skb, nhaddr, 0); + seg6_lookup_nexthop_noref(skb, nhaddr, 0); return dst_input(skb); } @@ -1383,7 +1406,7 @@ static int input_action_end_dt6(struct sk_buff *skb, /* note: this time we do not need to specify the table because the VRF * takes care of selecting the correct table. */ - seg6_lookup_any_nexthop(skb, NULL, 0, true, 0); + seg6_lookup_any_nexthop_noref(skb, NULL, 0, true, 0); return dst_input(skb); @@ -1391,7 +1414,7 @@ static int input_action_end_dt6(struct sk_buff *skb, #endif skb_set_transport_header(skb, sizeof(struct ipv6hdr)); - seg6_lookup_any_nexthop(skb, NULL, slwt->table, true, 0); + seg6_lookup_any_nexthop_noref(skb, NULL, slwt->table, true, 0); return dst_input(skb); @@ -1446,7 +1469,7 @@ static int input_action_end_b6(struct sk_buff *skb, struct seg6_local_lwt *slwt) skb_set_transport_header(skb, sizeof(struct ipv6hdr)); - seg6_lookup_nexthop(skb, NULL, 0); + seg6_lookup_nexthop_noref(skb, NULL, 0); return dst_input(skb); @@ -1482,7 +1505,7 @@ static int input_action_end_b6_encap(struct sk_buff *skb, skb_set_transport_header(skb, sizeof(struct ipv6hdr)); - seg6_lookup_nexthop(skb, NULL, 0); + seg6_lookup_nexthop_noref(skb, NULL, 0); return dst_input(skb); @@ -1571,7 +1594,7 @@ static int input_action_end_bpf(struct sk_buff *skb, local_unlock_nested_bh(&seg6_bpf_srh_states.bh_lock); if (ret != BPF_REDIRECT) - seg6_lookup_nexthop(skb, NULL, 0); + seg6_lookup_nexthop_noref(skb, NULL, 0); return dst_input(skb); -- 2.50.1 ip_route_input() looks the input route up with ip_route_input_noref() and then forces a reference on it, which is dropped again when the packet is transmitted. End.DX4 and End.DT4 call it and hand the skb to dst_input() within one RCU read-side section: the one of the receive path, or that of the nfnetlink verdict handler for a packet reinjected from NFQUEUE. Use ip_route_input_noref(), as ip_rcv_finish() does. Throughput at 0.5% packet loss of 82-byte IPv4-in-IPv6 UDP packets (no SRH), spread by RSS on an ixgbe 82599ES across two 10-core Xeon E5-2650 v3, offered by TRex over 10 runs of 10 s, in Mpps: End.DX4 queues cores before after 1 1 0.791 0.811 (+2.5%) 10 10 on the NIC's socket 6.118 7.394 (+20.9%) 16 10 + 6 on two sockets 3.305 10.205 (+208.8%) End.DT4 queues cores before after 1 1 0.481 0.489 (+1.6%) 10 10 on the NIC's socket 4.328 4.588 (+6.0%) 16 10 + 6 on two sockets 3.351 7.153 (+113.5%) Every CPU takes and drops a reference on the same input route, the one cached on the nexthop. At 16 queues that cache line bounces between the sockets, and skb_dst_force() and dst_release() together take more than half of the cycles for End.DX4 and over 40% for End.DT4. Signed-off-by: Yuya Kusakabe --- net/ipv6/seg6_local.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c index 7fbcf590297d..209e31c21e33 100644 --- a/net/ipv6/seg6_local.c +++ b/net/ipv6/seg6_local.c @@ -1096,7 +1096,7 @@ static int input_action_end_dx4_finish(struct net *net, struct sock *sk, skb_dst_drop(skb); - reason = ip_route_input(skb, nhaddr, iph->saddr, 0, skb->dev); + reason = ip_route_input_noref(skb, nhaddr, iph->saddr, 0, skb->dev); if (reason) { kfree_skb_reason(skb, reason); return -EINVAL; @@ -1322,7 +1322,7 @@ static int input_action_end_dt4(struct sk_buff *skb, iph = ip_hdr(skb); - reason = ip_route_input(skb, iph->daddr, iph->saddr, 0, skb->dev); + reason = ip_route_input_noref(skb, iph->daddr, iph->saddr, 0, skb->dev); if (unlikely(reason)) goto drop; -- 2.50.1